From 55f1738d50fb10a7d64caffc4ada6e0cc6cc24c7 Mon Sep 17 00:00:00 2001 From: Hiroshi Tanaka Date: Fri, 21 Aug 2026 00:50:24 +0900 Subject: [PATCH] fix(control-ui): config form save corrupts 64-bit id strings in string|number fields (#126402) * fix(control-ui): stop config form save from corrupting 64-bit id strings Saving the schema-driven config form coerced every numeric-looking string to a JS number before submission. For union-typed fields such as tools.elevated.allowFrom.* (anyOf: string | number), string entries holding 64-bit ids (Discord/Telegram snowflakes) were rewritten through Number(), which rounds past 2^53: "1048113311314608148" -> 1048113311314608100. The corruption also hit untouched fields, because serialization coerces the whole form, so merely saving an unrelated setting silently broke elevated-approval allowlists (fail-closed: the real user id no longer matched). Two guards fix this: - coerceFormValues keeps a string that already satisfies a string variant of an anyOf/oneOf union instead of parsing it into another variant's number. - coerceConfigFormNumberString refuses lossy integer parses: plain integer text beyond Number.MAX_SAFE_INTEGER that does not round-trip through BigInt stays a string, so pure number/integer fields fail validation loudly instead of storing a corrupted id. Co-Authored-By: Claude Fable 5 * fix(control-ui): harden 64-bit config id preservation * fix(control-ui): validate mixed-union scalar branches * test(control-ui): prove real gateway id preservation * test(control-ui): use communications route for config proof * test(control-ui): grant config proof admin scope * test(control-ui): reopen raw config for proof * fix(control-ui): preserve explicit union input types * test(control-ui): exercise union collection draft * ci: retry flaky control ui e2e * fix(control-ui): preserve mixed scalar branch types * ci: retry service worker e2e * fix(control-ui): preserve typeless string union branches * fix(control-ui): reject lossy decimal coercion * fix(control-ui): reject lossy pure numeric input * fix(control-ui): preserve exact numeric branch semantics * ci: retry checkout rate limit * ci(control-ui): capture real gateway proof * test(control-ui): frame config proof values * ci: retry checkout download * test(control-ui): prove Gateway-served production bundle * fix(control-ui): preserve exact incremental union edits * refactor(control-ui): isolate scalar edit session state * fix(control-ui): keep scalar edit branch type internal * fix(control-ui): avoid detached focus selector * fix(control-ui): round-trip exact numeric branches * refactor(control-ui): share exact scalar formatting --------- Co-authored-by: Claude Fable 5 --- .github/workflows/ci.yml | 20 ++ ...onfig-form-array-integrity.browser.test.ts | 49 +++ .../config-form-collection-draft.ts | 31 +- ...nfig-form-scalar-integrity.browser.test.ts | 339 ++++++++++++++++++ .../config-form.constraints.test.ts | 44 ++- ui/src/components/config-form.node.scalar.ts | 220 ++++++++++-- ui/src/components/config-form.node.shared.ts | 11 +- ui/src/components/config-form.numeric.ts | 82 ++++- ui/src/components/config-form.scalar-edit.ts | 84 +++++ ui/src/components/config-form.shared.ts | 23 ++ ui/src/e2e/config-safe-write.e2e.test.ts | 87 ++++- .../control-ui-auth-transports.e2e.test.ts | 164 ++++++++- ui/src/lib/config/config-draft-model.test.ts | 96 ++++- ui/src/lib/config/config-draft-model.ts | 12 +- 14 files changed, 1193 insertions(+), 69 deletions(-) create mode 100644 ui/src/components/config-form.scalar-edit.ts diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index fe35960ffc4f..ecd70ba1a6d5 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -8,6 +8,11 @@ on: required: false default: "" type: string + capture_ui_proof: + description: Capture and upload sanitized Control UI screenshots from real-Gateway tests. + required: false + default: false + type: boolean include_android: description: Run Android lanes for this manual CI dispatch. required: false @@ -1746,6 +1751,9 @@ jobs: - *cache_playwright_chromium - *install_playwright_chromium + - name: Build Control UI bundle for real-Gateway tests + run: pnpm ui:build + - name: Test MCP app conformance with a real Gateway run: >- node scripts/run-vitest.mjs run @@ -1754,12 +1762,24 @@ jobs: ui/src/e2e/mcp-app-conformance.e2e.test.ts - name: Test Control UI auth transports with a real Gateway + env: + OPENCLAW_CAPTURE_UI_PROOF: ${{ github.event_name == 'workflow_dispatch' && inputs.capture_ui_proof && '1' || '0' }} + OPENCLAW_UI_E2E_ARTIFACT_DIR: .artifacts/control-ui-e2e/real-gateway run: >- node scripts/run-vitest.mjs run --config test/vitest/vitest.ui-e2e.config.ts --configLoader runner ui/src/e2e/control-ui-auth-transports.e2e.test.ts + - name: Upload sanitized Control UI real-Gateway proof + if: always() && github.event_name == 'workflow_dispatch' && inputs.capture_ui_proof + uses: actions/upload-artifact@043fb46d1a93c77aae656e7c1c64a875d1fc6a0a # v7 + with: + name: control-ui-real-gateway-proof-${{ github.run_id }}-${{ github.run_attempt }} + path: .artifacts/control-ui-e2e/real-gateway + if-no-files-found: error + retention-days: 14 + - name: Test Control UI Logs lifecycle with a real Gateway run: >- node scripts/run-vitest.mjs run diff --git a/ui/src/components/config-form-array-integrity.browser.test.ts b/ui/src/components/config-form-array-integrity.browser.test.ts index e8e5aedba9c3..0d93fcc2ab97 100644 --- a/ui/src/components/config-form-array-integrity.browser.test.ts +++ b/ui/src/components/config-form-array-integrity.browser.test.ts @@ -134,6 +134,55 @@ describe("config form array integrity", () => { expect(onPatch).toHaveBeenCalledWith(["values"], []); }); + it("preserves unquoted strings and decodes quoted strings in string-number arrays", async () => { + const onPatch = vi.fn(); + const container = document.createElement("div"); + document.body.append(container); + const identifier = "1048113311314608148"; + renderArrayFixture(container, { + schema: { + type: "array", + items: { + oneOf: [{ enum: [identifier] }, { type: "number" }], + }, + }, + value: [], + path: ["allowFrom"], + onPatch, + }); + + const draftHost = expectElement( + container.querySelector("openclaw-config-form-collection-draft"), + "string-number array draft", + ); + const addDraftValue = async (value: string) => { + expectElement(findAddButton(container), "string-number array add").click(); + await draftHost.updateComplete; + const draftValue = expectElement( + draftHost.querySelector( + "[data-collection-draft-value]", + ), + "string-number array draft value", + ); + draftValue.value = value; + draftValue.dispatchEvent(new Event("input", { bubbles: true })); + await draftHost.updateComplete; + expectElement(findAddButton(draftHost), "string-number array draft commit").click(); + await draftHost.updateComplete; + }; + + await addDraftValue(identifier); + expect(onPatch).toHaveBeenCalledWith(["allowFrom"], [identifier]); + expect(onPatch.mock.calls[0]?.[1]?.[0]).not.toBe(Number(identifier)); + onPatch.mockClear(); + await addDraftValue(JSON.stringify(identifier)); + expect(onPatch).toHaveBeenCalledWith(["allowFrom"], [identifier]); + onPatch.mockClear(); + await addDraftValue("1e5"); + expect(onPatch).toHaveBeenCalledWith(["allowFrom"], [100_000]); + container.remove(); + }); + it("keeps large and unique array minimums incrementally editable", async () => { const onPatch = vi.fn(); const container = document.createElement("div"); diff --git a/ui/src/components/config-form-collection-draft.ts b/ui/src/components/config-form-collection-draft.ts index 11730cdf1075..ee283b3bf783 100644 --- a/ui/src/components/config-form-collection-draft.ts +++ b/ui/src/components/config-form-collection-draft.ts @@ -3,7 +3,8 @@ import { property, state } from "lit/decorators.js"; import { t } from "../i18n/index.ts"; import { OpenClawLightDomElement } from "../lit/openclaw-element.ts"; import { configValuesEqual, isSupportedConfigValueValid } from "./config-form.constraints.ts"; -import { schemaType, type JsonSchema } from "./config-form.shared.ts"; +import { coerceConfigFormNumberString } from "./config-form.numeric.ts"; +import { schemaMayAcceptString, schemaType, type JsonSchema } from "./config-form.shared.ts"; export type ConfigFormCollectionDraftProps = { schema: JsonSchema; @@ -89,19 +90,37 @@ export class ConfigFormCollectionDraft extends OpenClawLightDomElement { return { ok: true, value: null }; } const valueType = schemaType(schema); + const variants = schema.anyOf ?? schema.oneOf ?? []; + const stringNumberUnion = + variants.some(schemaMayAcceptString) && + variants.some((variant) => ["number", "integer"].includes(schemaType(variant) ?? "")); if (valueType === "string") { return { ok: true, value: this.draftValue }; } if (valueType === "number" || valueType === "integer") { - const value = Number(this.draftValue); - return this.draftValue.trim() && Number.isFinite(value) - ? { ok: true, value } + const coerced = coerceConfigFormNumberString(this.draftValue, valueType === "integer"); + return typeof coerced === "number" + ? { ok: true, value: coerced } : { ok: false, message: t("configForm.invalidNumber") }; } try { - return { ok: true, value: JSON.parse(this.draftValue) }; + const parsed = JSON.parse(this.draftValue) as unknown; + if (typeof parsed === "number") { + const coerced = coerceConfigFormNumberString(this.draftValue, false); + if (typeof coerced === "number") { + return { ok: true, value: coerced }; + } + // JSON.parse has already rounded unsafe integer spellings. Preserve + // the source text only when the union accepts it as a string. + return stringNumberUnion && isSupportedConfigValueValid(schema, this.draftValue) + ? { ok: true, value: this.draftValue } + : { ok: false, message: t("configForm.invalidNumber") }; + } + return { ok: true, value: parsed }; } catch { - return { ok: false, message: t("configForm.invalidJson") }; + return stringNumberUnion && isSupportedConfigValueValid(schema, this.draftValue) + ? { ok: true, value: this.draftValue } + : { ok: false, message: t("configForm.invalidJson") }; } } diff --git a/ui/src/components/config-form-scalar-integrity.browser.test.ts b/ui/src/components/config-form-scalar-integrity.browser.test.ts index 8732e7af74b3..37196a7f3d7b 100644 --- a/ui/src/components/config-form-scalar-integrity.browser.test.ts +++ b/ui/src/components/config-form-scalar-integrity.browser.test.ts @@ -327,6 +327,246 @@ describe("config form scalar integrity", () => { ).toBe("Default: balanced"); }); + it("commits the valid branch type for constrained text unions", () => { + const container = document.createElement("div"); + const onPatch = vi.fn(); + render( + renderTextInput({ + schema: { + anyOf: [ + { type: "string", const: "auto" }, + { type: "integer", minimum: 0 }, + ], + }, + value: "auto", + path: ["mode"], + hints: {}, + unsupported: new Set(), + disabled: false, + inputType: "text", + onPatch, + }), + container, + ); + const input = expectElement( + container.querySelector("input[type='text']"), + "constrained union input", + ); + + input.value = "42"; + input.dispatchEvent(new Event("input", { bubbles: true })); + expect(onPatch).toHaveBeenLastCalledWith(["mode"], 42); + expect(input.getAttribute("aria-invalid")).toBe("false"); + + input.value = "auto"; + input.dispatchEvent(new Event("input", { bubbles: true })); + expect(onPatch).toHaveBeenLastCalledWith(["mode"], "auto"); + + onPatch.mockClear(); + input.value = "invalid"; + input.dispatchEvent(new Event("input", { bubbles: true })); + expect(onPatch).not.toHaveBeenCalled(); + expect(input.getAttribute("aria-invalid")).toBe("true"); + }); + + it("commits explicit boolean branches without retyping numeric strings", () => { + const container = document.createElement("div"); + const onPatch = vi.fn(); + render( + renderTextInput({ + schema: { + anyOf: [{ type: "string" }, { type: "number" }, { const: false }], + }, + value: "500mb", + path: ["maxDiskBytes"], + hints: {}, + unsupported: new Set(), + disabled: false, + inputType: "text", + onPatch, + }), + container, + ); + const input = expectElement( + container.querySelector("input[type='text']"), + "string-number-boolean union input", + ); + + input.value = "false"; + input.dispatchEvent(new Event("input", { bubbles: true })); + expect(onPatch).toHaveBeenLastCalledWith(["maxDiskBytes"], false); + + input.value = "true"; + input.dispatchEvent(new Event("input", { bubbles: true })); + expect(onPatch).toHaveBeenLastCalledWith(["maxDiskBytes"], "true"); + + const identifier = "1048113311314608148"; + input.value = identifier; + input.dispatchEvent(new Event("input", { bubbles: true })); + expect(onPatch).toHaveBeenLastCalledWith(["maxDiskBytes"], identifier); + }); + + it("preserves the current branch type in unconstrained primitive unions", () => { + const container = document.createElement("div"); + const onPatch = vi.fn(); + const schema = { + anyOf: [{ type: "string" }, { type: "number" }, { type: "boolean" }], + }; + const renderValue = (value: unknown, defaultValue?: unknown) => { + render( + renderTextInput({ + schema: defaultValue === undefined ? schema : { ...schema, default: defaultValue }, + value, + path: ["providerOptions", "deepgram", "temperature"], + hints: {}, + unsupported: new Set(), + disabled: false, + inputType: "text", + onPatch, + }), + container, + ); + return expectElement( + container.querySelector("input[type='text']"), + "mixed primitive union input", + ); + }; + + let input = renderValue(42); + input.value = "43"; + input.dispatchEvent(new Event("input", { bubbles: true })); + expect(onPatch).toHaveBeenLastCalledWith(["providerOptions", "deepgram", "temperature"], 43); + + onPatch.mockClear(); + input = renderValue(1); + input.value = "1.0000000000000001"; + input.dispatchEvent(new Event("input", { bubbles: true })); + input.dispatchEvent(new Event("change", { bubbles: true })); + expect(onPatch).not.toHaveBeenCalled(); + expect(input.getAttribute("aria-invalid")).toBe("true"); + expect(input.value).toBe("1.0000000000000001"); + + onPatch.mockClear(); + input = renderValue("42"); + input.value = "43"; + input.dispatchEvent(new Event("input", { bubbles: true })); + expect(onPatch).toHaveBeenLastCalledWith(["providerOptions", "deepgram", "temperature"], "43"); + + onPatch.mockClear(); + input = renderValue(undefined); + input.value = "43"; + input.dispatchEvent(new Event("input", { bubbles: true })); + expect(onPatch).toHaveBeenLastCalledWith(["providerOptions", "deepgram", "temperature"], 43); + + onPatch.mockClear(); + input = renderValue(undefined, 42); + input.value = "43"; + input.dispatchEvent(new Event("input", { bubbles: true })); + expect(onPatch).toHaveBeenLastCalledWith(["providerOptions", "deepgram", "temperature"], 43); + + onPatch.mockClear(); + input = renderValue(undefined, "42"); + input.value = "43"; + input.dispatchEvent(new Event("input", { bubbles: true })); + expect(onPatch).toHaveBeenLastCalledWith(["providerOptions", "deepgram", "temperature"], "43"); + + onPatch.mockClear(); + input = renderValue("false"); + input.value = "true"; + input.dispatchEvent(new Event("input", { bubbles: true })); + expect(onPatch).toHaveBeenLastCalledWith( + ["providerOptions", "deepgram", "temperature"], + "true", + ); + + onPatch.mockClear(); + input = renderValue(false); + input.value = "true"; + input.dispatchEvent(new Event("input", { bubbles: true })); + expect(onPatch).toHaveBeenLastCalledWith(["providerOptions", "deepgram", "temperature"], true); + + onPatch.mockClear(); + const identifier = "1048113311314608148"; + input = renderValue(undefined); + input.value = identifier; + input.dispatchEvent(new Event("input", { bubbles: true })); + expect(onPatch).toHaveBeenLastCalledWith( + ["providerOptions", "deepgram", "temperature"], + identifier, + ); + }); + + it.each([ + ["unset", undefined], + ["number", 0], + ] as const)( + "keeps an initial %s branch stable while an identifier is typed", + (_name, initial) => { + const container = document.createElement("div"); + document.body.append(container); + const identifier = "1048113311314608148"; + const schema = { + anyOf: [{ type: "string", pattern: "^[0-9]{19}$" }, { type: "number" }], + }; + const patches: unknown[] = []; + let persisted: unknown = initial; + let value: unknown = initial; + + const renderValue = () => { + render( + renderTextInput({ + schema, + value, + path: ["allowFrom"], + hints: {}, + unsupported: new Set(), + disabled: false, + inputType: "text", + onPatch: (_path, nextValue) => { + patches.push(nextValue); + persisted = nextValue; + value = nextValue; + // Model application immediately refreshes the rendered field. + renderValue(); + }, + }), + container, + ); + }; + + try { + renderValue(); + let input = expectElement( + container.querySelector("input[type='text']"), + "incremental string-number input", + ); + input.focus(); + input.value = ""; + for (const [index, digit] of Array.from(identifier).entries()) { + input.value += digit; + input.dispatchEvent(new Event("input", { bubbles: true })); + // A background refresh can land even when the prefix is not yet a + // valid string branch; the focused edit must survive that repaint. + renderValue(); + input = expectElement( + container.querySelector("input[type='text']"), + `incremental string-number input ${index + 1}`, + ); + } + + expect(patches.length).toBeGreaterThan(1); + expect(patches.slice(0, -1).every((candidate) => typeof candidate === "number")).toBe(true); + expect(patches.at(-1)).toBe(identifier); + expect(persisted).toBe(identifier); + expect(value).toBe(identifier); + expect(input.value).toBe(identifier); + input.blur(); + } finally { + container.remove(); + } + }, + ); + it("does not commit a clear while a number input holds partial numeric text", () => { // Browsers report value === "" with validity.badInput while the user is // mid-keystroke ("0." on the way to "0.5"). Committing undefined here @@ -373,6 +613,105 @@ describe("config form scalar integrity", () => { expect(onPatch).toHaveBeenCalledWith(["sampleRate"], undefined); }); + it.each([ + ["unsafe integer", { type: "integer" }, "9007199254740993"], + ["lossy decimal", { type: "number" }, "1.0000000000000001"], + ["underflow", { type: "number" }, "1e-324"], + ])("rejects %s text before a pure numeric input can round it", (_name, schema, raw) => { + const container = document.createElement("div"); + const onPatch = vi.fn(); + render( + renderNumberInput({ + schema, + value: 0, + path: ["numeric"], + hints: {}, + unsupported: new Set(), + disabled: false, + onPatch, + }), + container, + ); + const input = expectElement( + container.querySelector("input[type='number']"), + "lossless number input", + ); + + input.value = raw; + input.dispatchEvent(new Event("input", { bubbles: true })); + input.dispatchEvent(new Event("change", { bubbles: true })); + + expect(onPatch).not.toHaveBeenCalled(); + expect(input.getAttribute("aria-invalid")).toBe("true"); + expect(input.value).toBe(raw); + }); + + it("accepts an exactly represented integer above the safe-integer range", () => { + const container = document.createElement("div"); + const onPatch = vi.fn(); + render( + renderNumberInput({ + schema: { type: "integer" }, + value: 0, + path: ["numeric"], + hints: {}, + unsupported: new Set(), + disabled: false, + onPatch, + }), + container, + ); + const input = expectElement( + container.querySelector("input[type='number']"), + "exact large number input", + ); + + input.value = "9007199254740992"; + input.dispatchEvent(new Event("input", { bubbles: true })); + + expect(onPatch).toHaveBeenCalledWith(["numeric"], 9_007_199_254_740_992); + expect(input.getAttribute("aria-invalid")).toBe("false"); + }); + + it.each(["mixed", "number"] as const)( + "renders an exact large integer as parser-valid text in a %s input", + (kind) => { + const container = document.createElement("div"); + const onPatch = vi.fn(); + const exactValue = Number("1000000000000000128"); + const params = { + value: exactValue, + path: ["numeric"], + hints: {}, + unsupported: new Set(), + disabled: false, + onPatch, + }; + render( + kind === "mixed" + ? renderTextInput({ + ...params, + schema: { anyOf: [{ type: "string" }, { type: "number" }] }, + inputType: "text", + }) + : renderNumberInput({ ...params, schema: { type: "integer" } }), + container, + ); + const input = expectElement( + container.querySelector( + `input[type='${kind === "mixed" ? "text" : "number"}']`, + ), + `${kind} exact large number input`, + ); + + expect(input.value).toBe("1000000000000000128"); + input.dispatchEvent(new Event("input", { bubbles: true })); + + expect(onPatch).toHaveBeenCalledWith(["numeric"], exactValue); + expect(input.getAttribute("aria-invalid")).toBe("false"); + }, + ); + it("keeps restore disabled while a sensitive value is concealed", () => { const container = document.createElement("div"); diff --git a/ui/src/components/config-form.constraints.test.ts b/ui/src/components/config-form.constraints.test.ts index 15a5f790ca0b..bfa503b1607d 100644 --- a/ui/src/components/config-form.constraints.test.ts +++ b/ui/src/components/config-form.constraints.test.ts @@ -13,7 +13,7 @@ import { objectPropertySchema, requiredPropertyKeys, } from "./config-form.constraints.ts"; -import { coerceConfigFormNumberString } from "./config-form.numeric.ts"; +import { coerceConfigFormNumberString, formatConfigFormNumber } from "./config-form.numeric.ts"; import type { JsonSchema } from "./config-form.shared.ts"; describe("config form schema constraints", () => { @@ -23,6 +23,30 @@ describe("config form schema constraints", () => { expect(coerceConfigFormNumberString("-2.5E-3", false)).toBe(-0.0025); expect(coerceConfigFormNumberString("1e5", true)).toBe(100_000); expect(coerceConfigFormNumberString("", false)).toBeUndefined(); + expect(coerceConfigFormNumberString("9007199254740991", true)).toBe(Number.MAX_SAFE_INTEGER); + expect(coerceConfigFormNumberString("9.007199254740991e15", true)).toBe( + Number.MAX_SAFE_INTEGER, + ); + expect(coerceConfigFormNumberString("9007199254740992", true)).toBe(9_007_199_254_740_992); + expect(coerceConfigFormNumberString("9007199254740993", true)).toBe("9007199254740993"); + expect(coerceConfigFormNumberString("-9007199254740993", false)).toBe("-9007199254740993"); + expect(coerceConfigFormNumberString("9007199254740992.0", true)).toBe(9_007_199_254_740_992); + expect(coerceConfigFormNumberString("9007199254740993.0", true)).toBe("9007199254740993.0"); + expect(coerceConfigFormNumberString("10481133113146081487e0", true)).toBe( + "10481133113146081487e0", + ); + expect(coerceConfigFormNumberString("9.007199254740993e15", true)).toBe("9.007199254740993e15"); + expect(coerceConfigFormNumberString("9.007199254740992e15", true)).toBe(9_007_199_254_740_992); + expect(coerceConfigFormNumberString("-9.007199254740993e15", true)).toBe( + "-9.007199254740993e15", + ); + expect(coerceConfigFormNumberString("0.10", false)).toBe(0.1); + expect(coerceConfigFormNumberString("1.0000000000000002", false)).toBe(1.0000000000000002); + expect(coerceConfigFormNumberString("1.0000000000000001", false)).toBe("1.0000000000000001"); + expect(coerceConfigFormNumberString("9007199254740991.1", false)).toBe("9007199254740991.1"); + expect(coerceConfigFormNumberString("1e-324", false)).toBe("1e-324"); + expect(coerceConfigFormNumberString("0e-1025", false)).toBe(0); + expect(Object.is(coerceConfigFormNumberString("-0e2000", false), -0)).toBe(true); for (const spelling of [ "0x10", @@ -40,6 +64,24 @@ describe("config form schema constraints", () => { expect(coerceConfigFormNumberString("42.5", true)).toBe("42.5"); }); + it("compares integer spellings against the exact binary double", () => { + const exactLargeInteger = Number("1000000000000000128"); + expect(coerceConfigFormNumberString("1000000000000000100", true)).toBe("1000000000000000100"); + expect(coerceConfigFormNumberString("1000000000000000100", false)).toBe("1000000000000000100"); + expect(coerceConfigFormNumberString("-1000000000000000100", false)).toBe( + "-1000000000000000100", + ); + expect(coerceConfigFormNumberString("1000000000000000127", true)).toBe("1000000000000000127"); + expect(coerceConfigFormNumberString("1000000000000000128", true)).toBe(exactLargeInteger); + expect(formatConfigFormNumber(exactLargeInteger)).toBe("1000000000000000128"); + expect(formatConfigFormNumber(-0)).toBe("0"); + expect(formatConfigFormNumber(0.1)).toBe("0.1"); + expect(coerceConfigFormNumberString(formatConfigFormNumber(exactLargeInteger), true)).toBe( + exactLargeInteger, + ); + expect(coerceConfigFormNumberString("9007199254740994", true)).toBe(9_007_199_254_740_994); + }); + it("rejects non-finite decimal rationals and schema multiples", () => { expect(numericInputConstraints({ type: "number", multipleOf: Number.NaN }).step).toBe("any"); expect( diff --git a/ui/src/components/config-form.node.scalar.ts b/ui/src/components/config-form.node.scalar.ts index 0eae9239bf7c..38967ed768e6 100644 --- a/ui/src/components/config-form.node.scalar.ts +++ b/ui/src/components/config-form.node.scalar.ts @@ -3,7 +3,6 @@ import { formatInternationalPhoneNumberForDisplay } from "@openclaw/normalizatio import { html, nothing, type TemplateResult } from "lit"; import { ref } from "lit/directives/ref.js"; import { i18n, t } from "../i18n/index.ts"; -import { formatUnknownText } from "../lib/format.ts"; import { isSupportedConfigValueValid, normalizeNumericValue, @@ -11,6 +10,7 @@ import { } from "./config-form.constraints.ts"; import { configEnumOptionLabel, + formatConfigValueText, getSensitiveRenderState, isSecretRefObject, jsonValue, @@ -21,8 +21,27 @@ import { wrapSensitiveControl, type ConfigNodeRenderParams, } from "./config-form.node.shared.ts"; +import { + coerceConfigFormNumberString, + isConfigFormDecimalNumberString, + isConfigFormUnsafeIntegerString, +} from "./config-form.numeric.ts"; +import { + beginScalarEdit, + finishScalarEdit, + finishScalarEditFromEvent, + scalarEditHintForInput, + scalarValueBranch, + syncScalarEditIdentity, + type ScalarEditHint, +} from "./config-form.scalar-edit.ts"; import { resolveConfigFieldMeta as resolveFieldMeta } from "./config-form.search.ts"; -import { configFieldId, hintForPath, redactedPlaceholder } from "./config-form.shared.ts"; +import { + configFieldId, + hintForPath, + redactedPlaceholder, + schemaType, +} from "./config-form.shared.ts"; const scalarInputState = new WeakMap< HTMLInputElement, @@ -89,16 +108,98 @@ function syncScalarInputIdentity( }); } -function stringConstraintMessage(value: string, schema: ConfigNodeRenderParams["schema"]): string { - return isSupportedConfigValueValid(schema, value) ? "" : t("configForm.invalidString"); +function coerceTextInputValue( + value: string, + schema: ConfigNodeRenderParams["schema"], + currentValue?: unknown, + editHint?: ScalarEditHint, +): string | number | boolean | undefined { + const trimmed = value.trim(); + const variants = schema.anyOf ?? schema.oneOf ?? []; + const stringCandidateValid = isSupportedConfigValueValid(schema, value); + const currentBranch = editHint ? editHint.branch : scalarValueBranch(currentValue); + const booleanCandidate = trimmed === "true" ? true : trimmed === "false" ? false : undefined; + if (booleanCandidate !== undefined && isSupportedConfigValueValid(schema, booleanCandidate)) { + let booleanBranchValid = false; + let explicitBooleanBranchValid = false; + for (const variant of variants) { + const booleanBranch = + schemaType(variant) === "boolean" || + typeof variant.const === "boolean" || + variant.enum?.some((entry) => typeof entry === "boolean"); + if (!booleanBranch || !isSupportedConfigValueValid(variant, booleanCandidate)) { + continue; + } + booleanBranchValid = true; + explicitBooleanBranchValid ||= + Object.is(variant.const, booleanCandidate) || + Boolean(variant.enum?.some((entry) => Object.is(entry, booleanCandidate))); + } + if ( + booleanBranchValid && + (currentBranch !== "string" || explicitBooleanBranchValid || !stringCandidateValid) + ) { + return booleanCandidate; + } + } + let numberCandidate: number | undefined; + for (const variant of variants) { + const type = schemaType(variant); + if (type !== "number" && type !== "integer") { + continue; + } + const candidate = coerceConfigFormNumberString(value, type === "integer"); + if (typeof candidate === "number" && isSupportedConfigValueValid(schema, candidate)) { + numberCandidate = candidate; + break; + } + } + if (currentBranch === "number") { + if (numberCandidate !== undefined) { + return numberCandidate; + } + if (isConfigFormDecimalNumberString(value)) { + return stringCandidateValid && isConfigFormUnsafeIntegerString(trimmed) ? value : undefined; + } + } + if (currentBranch === "string" && stringCandidateValid) { + return value; + } + if (numberCandidate !== undefined) { + return numberCandidate; + } + if (stringCandidateValid) { + return value; + } + return value; +} + +function stringConstraintMessage( + value: string, + schema: ConfigNodeRenderParams["schema"], + currentValue?: unknown, + editHint?: ScalarEditHint, +): string { + return isSupportedConfigValueValid( + schema, + coerceTextInputValue(value, schema, currentValue, editHint), + ) + ? "" + : t("configForm.invalidString"); } function shouldClearOptionalEmpty( value: string, schema: ConfigNodeRenderParams["schema"], isRequired: boolean, + currentValue?: unknown, + editHint?: ScalarEditHint, ): boolean { - return value === "" && !isRequired && Boolean(stringConstraintMessage(value, schema)); + return ( + value === "" && + !isRequired && + Boolean(stringConstraintMessage(value, schema, currentValue, editHint)) + ); } function numericConstraintMessage(value: number, schema: ConfigNodeRenderParams["schema"]): string { @@ -108,6 +209,7 @@ function numericConstraintMessage(value: number, schema: ConfigNodeRenderParams[ type NumericInputState = | { kind: "badInput" } | { kind: "empty" } + | { kind: "invalid" } | { kind: "value"; parsed: number; message: string }; // Partial numeric text ("3.", "-", "1e") reports value === "" with @@ -121,7 +223,10 @@ function resolveNumericInputState( if (raw.trim() === "") { return target.validity.badInput ? { kind: "badInput" } : { kind: "empty" }; } - const parsed = Number(raw); + const parsed = coerceConfigFormNumberString(raw, schemaType(schema) === "integer"); + if (typeof parsed !== "number") { + return { kind: "invalid" }; + } return { kind: "value", parsed, message: numericConstraintMessage(parsed, schema) }; } @@ -129,6 +234,9 @@ function numericStateMessage(state: NumericInputState, isRequired: boolean): str if (state.kind === "value") { return state.message; } + if (state.kind === "invalid") { + return t("configForm.invalidNumber"); + } return state.kind === "badInput" || isRequired ? t("configForm.invalidNumber") : ""; } @@ -144,7 +252,7 @@ function applyNumericInputState( if (state.kind === "empty") { commit(undefined); } else if (state.kind === "value") { - commit(Number.isNaN(state.parsed) ? target.value : state.parsed); + commit(state.parsed); } } @@ -184,13 +292,15 @@ export function renderTextInput( : redactedPlaceholder() : (hint?.placeholder ?? (schema.default !== undefined - ? t("configForm.defaultValue", { value: formatUnknownText(schema.default) }) + ? t("configForm.defaultValue", { value: formatConfigValueText(schema.default) }) : "")); const displayValue = effectiveRedacted ? "" : isStructuredValue ? jsonValue(value) : (value ?? ""); + const effectiveValue = value !== undefined ? value : schema.default; + const initialBranch = scalarValueBranch(effectiveValue); const effectiveInputType = sensitiveState.isSensitive && !effectiveRedacted ? "text" : inputType; const isPhonePresentation = hint?.presentation === "phone-number"; const phonePresentation = @@ -200,7 +310,7 @@ export function renderTextInput( const controlIdentity = params.controlIdentity ?? params.sourceIdentity ?? value; const sourceIdentity = params.sourceIdentity ?? value; const controlPathKey = configFieldId(path, "scalar-identity"); - const renderedValue = formatUnknownText(displayValue); + const renderedValue = formatConfigValueText(displayValue); const presentationIdentity = [ effectiveRedacted ? "redacted" : "visible", effectiveInputType, @@ -220,8 +330,18 @@ export function renderTextInput( return; } const raw = target.value; - const optionalEmpty = shouldClearOptionalEmpty(raw, schema, params.isRequired === true); - setControlValidity(target, optionalEmpty ? "" : stringConstraintMessage(raw, schema)); + const editHint = scalarEditHintForInput(target, initialBranch); + const optionalEmpty = shouldClearOptionalEmpty( + raw, + schema, + params.isRequired === true, + effectiveValue, + editHint, + ); + setControlValidity( + target, + optionalEmpty ? "" : stringConstraintMessage(raw, schema, effectiveValue, editHint), + ); }; const commitScalarValue = (target: HTMLInputElement, candidate: unknown) => { if (onPatch(path, candidate) !== false) { @@ -234,7 +354,8 @@ export function renderTextInput( const inputControl = html` + ${ref((element) => { + syncScalarEditIdentity(element, params.rowIdentity, controlPathKey, presentationIdentity); syncScalarInputIdentity( element, controlIdentity, @@ -244,8 +365,8 @@ export function renderTextInput( presentationIdentity, renderedValue, revalidate, - ), - )} + ); + })} type=${effectiveInputType} class="settings-input${effectiveRedacted ? " cfg-redacted" : ""}" aria-label=${label} @@ -275,11 +396,22 @@ export function renderTextInput( ); return; } - if (shouldClearOptionalEmpty(raw, schema, params.isRequired === true)) { + const editHint = beginScalarEdit(target, initialBranch); + if ( + shouldClearOptionalEmpty( + raw, + schema, + params.isRequired === true, + effectiveValue, + editHint, + ) + ) { setControlValidity(target, ""); commitScalarValue(target, undefined); - } else if (setControlValidity(target, stringConstraintMessage(raw, schema))) { - commitScalarValue(target, raw); + } else if ( + setControlValidity(target, stringConstraintMessage(raw, schema, effectiveValue, editHint)) + ) { + commitScalarValue(target, coerceTextInputValue(raw, schema, effectiveValue, editHint)); } }} @change=${(event: Event) => { @@ -287,29 +419,51 @@ export function renderTextInput( return; } const target = event.target as HTMLInputElement; + const editHint = beginScalarEdit(target, initialBranch); const raw = target.value; - const rawMessage = stringConstraintMessage(raw, schema); + const rawMessage = stringConstraintMessage(raw, schema, effectiveValue, editHint); if (!rawMessage && !isPhonePresentation) { setControlValidity(target, ""); - commitScalarValue(target, raw); + commitScalarValue(target, coerceTextInputValue(raw, schema, effectiveValue, editHint)); + finishScalarEdit(target); return; } const normalized = raw.trim(); - if (shouldClearOptionalEmpty(normalized, schema, params.isRequired === true)) { + if ( + shouldClearOptionalEmpty( + normalized, + schema, + params.isRequired === true, + effectiveValue, + editHint, + ) + ) { target.value = normalized; setControlValidity(target, ""); commitScalarValue(target, undefined); + finishScalarEdit(target); return; } - const normalizedMessage = stringConstraintMessage(normalized, schema); + const normalizedMessage = stringConstraintMessage( + normalized, + schema, + effectiveValue, + editHint, + ); if (normalizedMessage) { setControlValidity(target, rawMessage); + finishScalarEdit(target); return; } target.value = normalized; setControlValidity(target, ""); - commitScalarValue(target, normalized); + commitScalarValue( + target, + coerceTextInputValue(normalized, schema, effectiveValue, editHint), + ); + finishScalarEdit(target); }} + @blur=${finishScalarEditFromEvent} /> `; const revealToggle = isStructuredSecretRef @@ -362,7 +516,7 @@ export function renderNumberInput(params: ConfigNodeRenderParams): TemplateResul const controlIdentity = params.controlIdentity ?? params.sourceIdentity ?? value; const sourceIdentity = params.sourceIdentity ?? value; const controlPathKey = configFieldId(path, "scalar-identity"); - const renderedValue = formatUnknownText(displayValue); + const renderedValue = formatConfigValueText(displayValue); const revalidate = (target: HTMLInputElement) => { setControlValidity( target, @@ -420,7 +574,7 @@ export function renderNumberInput(params: ConfigNodeRenderParams): TemplateResul aria-describedby=${helpId ?? nothing} aria-invalid="false" placeholder=${schema.default !== undefined - ? t("configForm.defaultValue", { value: formatUnknownText(schema.default) }) + ? t("configForm.defaultValue", { value: formatConfigValueText(schema.default) }) : nothing} min=${constraints.min ?? nothing} max=${constraints.max ?? nothing} @@ -448,19 +602,13 @@ export function renderNumberInput(params: ConfigNodeRenderParams): TemplateResul }} @change=${(event: Event) => { const target = event.target as HTMLInputElement; - if (target.value === "") { - if (target.validity.badInput) { - setControlValidity(target, t("configForm.invalidNumber")); - } + const state = resolveNumericInputState(target, schema); + if (state.kind !== "value") { + setControlValidity(target, numericStateMessage(state, params.isRequired === true)); return; } - const parsed = Number(target.value); - if (!Number.isFinite(parsed)) { - setControlValidity(target, t("configForm.invalidNumber")); - return; - } - const normalized = normalizeNumericValue(parsed, schema); - target.value = formatUnknownText(normalized); + const normalized = normalizeNumericValue(state.parsed, schema); + target.value = formatConfigValueText(normalized); if (setControlValidity(target, numericConstraintMessage(normalized, schema))) { commitScalarValue(target, normalized); } @@ -550,7 +698,7 @@ export function renderSelect( ?disabled=${params.isRequired && schema.default === undefined} > ${schema.default !== undefined - ? t("configForm.defaultValue", { value: formatUnknownText(schema.default) }) + ? t("configForm.defaultValue", { value: formatConfigValueText(schema.default) }) : t("configForm.select")} ${canSelectNull diff --git a/ui/src/components/config-form.node.shared.ts b/ui/src/components/config-form.node.shared.ts index bd775e06f0e8..aef93528cfee 100644 --- a/ui/src/components/config-form.node.shared.ts +++ b/ui/src/components/config-form.node.shared.ts @@ -9,6 +9,7 @@ import "../components/tooltip.ts"; import { REDACTED_SENTINEL } from "../lib/config-form-utils.ts"; import { formatUnknownText } from "../lib/format.ts"; import { configValuesEqual, isSupportedConfigValueValid } from "./config-form.constraints.ts"; +import { formatConfigFormNumber } from "./config-form.numeric.ts"; import type { ConfigSearchCriteria } from "./config-form.search.ts"; import { configFieldId, @@ -84,6 +85,10 @@ export function jsonValue(value: unknown): string { } } +export function formatConfigValueText(value: unknown): string { + return typeof value === "number" ? formatConfigFormNumber(value) : formatUnknownText(value); +} + export function schemaWithDefault(schema: JsonSchema, value: unknown): JsonSchema { return { ...schema, default: value }; } @@ -315,7 +320,7 @@ export function renderSchemaDefaultDescription( return nothing; } return html`${t(value === undefined ? "configForm.usingDefault" : "configForm.defaultValue", { - value: formatUnknownText(schema.default), + value: formatConfigValueText(schema.default), })}`; } @@ -384,7 +389,7 @@ export function renderSegmentedControl(params: { export function configEnumOptionLabel(option: unknown, options: readonly unknown[]): string { const presentsBooleanState = options.includes(true) && options.includes(false); if (!presentsBooleanState) { - return formatUnknownText(option); + return formatConfigValueText(option); } if (option === true) { return t("configForm.enumOn"); @@ -392,7 +397,7 @@ export function configEnumOptionLabel(option: unknown, options: readonly unknown if (option === false) { return t("configForm.enumOff"); } - return option === "auto" ? t("configForm.enumAuto") : formatUnknownText(option); + return option === "auto" ? t("configForm.enumAuto") : formatConfigValueText(option); } export function renderJsonTextareaControl(params: { diff --git a/ui/src/components/config-form.numeric.ts b/ui/src/components/config-form.numeric.ts index 661b19d436b5..ebac9d86b4a9 100644 --- a/ui/src/components/config-form.numeric.ts +++ b/ui/src/components/config-form.numeric.ts @@ -4,6 +4,53 @@ type DecimalRational = { }; const CONFIG_FORM_DECIMAL_NUMBER_RE = /^-?(?:\d+(?:\.\d*)?|\.\d+)(?:[eE][+-]?\d+)?$/u; +const MAX_CONFIG_FORM_DECIMAL_RATIONAL_DIGITS = 1024; + +function decimalStringRational(value: string): DecimalRational | undefined { + if (!CONFIG_FORM_DECIMAL_NUMBER_RE.test(value)) { + return undefined; + } + const [coefficientText = "", exponentText] = value.toLowerCase().split("e"); + const negative = coefficientText.startsWith("-"); + const coefficient = negative ? coefficientText.slice(1) : coefficientText; + const [wholeText = "", fraction = ""] = coefficient.split("."); + const whole = wholeText || "0"; + const digitsText = `${whole}${fraction}`; + if (/^0+$/u.test(digitsText)) { + return { numerator: 0n, denominator: 1n }; + } + const exponent = Number(exponentText ?? 0); + if (!Number.isSafeInteger(exponent)) { + return undefined; + } + const fractionalPlaces = fraction.length - exponent; + if ( + digitsText.length > MAX_CONFIG_FORM_DECIMAL_RATIONAL_DIGITS || + Math.abs(fractionalPlaces) > MAX_CONFIG_FORM_DECIMAL_RATIONAL_DIGITS + ) { + return undefined; + } + const digits = BigInt(digitsText); + const numerator = fractionalPlaces < 0 ? digits * 10n ** BigInt(-fractionalPlaces) : digits; + return { + numerator: negative ? -numerator : numerator, + denominator: fractionalPlaces > 0 ? 10n ** BigInt(fractionalPlaces) : 1n, + }; +} + +function decimalRationalsEqual(left: DecimalRational, right: DecimalRational): boolean { + return left.numerator * right.denominator === right.numerator * left.denominator; +} + +export function isConfigFormDecimalNumberString(value: string): boolean { + const trimmed = value.trim(); + return trimmed !== "" && CONFIG_FORM_DECIMAL_NUMBER_RE.test(trimmed); +} + +export function isConfigFormUnsafeIntegerString(value: string): boolean { + const trimmed = value.trim(); + return /^-?\d+$/u.test(trimmed) && !Number.isSafeInteger(Number(trimmed)); +} export function coerceConfigFormNumberString( value: string, @@ -13,30 +60,39 @@ export function coerceConfigFormNumberString( if (trimmed === "") { return undefined; } - if (!CONFIG_FORM_DECIMAL_NUMBER_RE.test(trimmed)) { + if (!isConfigFormDecimalNumberString(trimmed)) { return value; } const parsed = Number(trimmed); if (!Number.isFinite(parsed) || (integer && !Number.isInteger(parsed))) { return value; } + const authored = decimalStringRational(trimmed); + if (!authored) { + return value; + } + const decimalSpelling = Number.isInteger(parsed) ? undefined : decimalRational(parsed); + // Integer-valued doubles need bit-exact comparison: shortest-decimal output + // can hide a rounded integer. Fractional values retain decimal-spelling + // comparison so ordinary JSON decimals such as 0.10 keep their old type. + const matchesRepresentedValue = Number.isInteger(parsed) + ? authored.numerator === BigInt(parsed) * authored.denominator + : decimalSpelling && decimalRationalsEqual(authored, decimalSpelling); + if (!matchesRepresentedValue) { + return value; + } return parsed; } +export function formatConfigFormNumber(value: number): string { + return Number.isInteger(value) ? BigInt(value).toString() : String(value); +} + +// Keep this decimal-spelling form for JSON Schema step arithmetic; scalar +// integer coercion uses BigInt above to detect hidden rounding. export function decimalRational(value: number): DecimalRational | undefined { if (!Number.isFinite(value)) { return undefined; } - const [coefficientText = "", exponentText] = String(value).toLowerCase().split("e"); - const negative = coefficientText.startsWith("-"); - const coefficient = negative ? coefficientText.slice(1) : coefficientText; - const [whole = "0", fraction = ""] = coefficient.split("."); - const exponent = Number(exponentText ?? 0); - const digits = BigInt(`${whole}${fraction}`); - const fractionalPlaces = fraction.length - exponent; - const numerator = fractionalPlaces < 0 ? digits * 10n ** BigInt(-fractionalPlaces) : digits; - return { - numerator: negative ? -numerator : numerator, - denominator: fractionalPlaces > 0 ? 10n ** BigInt(fractionalPlaces) : 1n, - }; + return decimalStringRational(String(value)); } diff --git a/ui/src/components/config-form.scalar-edit.ts b/ui/src/components/config-form.scalar-edit.ts new file mode 100644 index 000000000000..c161e86d927b --- /dev/null +++ b/ui/src/components/config-form.scalar-edit.ts @@ -0,0 +1,84 @@ +// Scalar edit sessions keep their initial primitive branch while focused rerenders apply patches. +type ScalarValueBranch = "string" | "number" | "boolean"; + +export type ScalarEditHint = { + branch?: ScalarValueBranch; +}; + +type ScalarEditState = { + edit?: ScalarEditHint; + pathKey: string; + presentationIdentity: string; + rowIdentity: unknown; +}; + +const scalarEditState = new WeakMap(); + +export function scalarValueBranch(value: unknown): ScalarValueBranch | undefined { + if (typeof value === "string") { + return "string"; + } + if (typeof value === "number") { + return "number"; + } + if (typeof value === "boolean") { + return "boolean"; + } + return undefined; +} + +export function syncScalarEditIdentity( + element: Element | undefined, + rowIdentity: unknown, + pathKey: string, + presentationIdentity: string, +): void { + if (!(element instanceof HTMLInputElement)) { + return; + } + const previous = scalarEditState.get(element); + const preserveEdit = + previous?.edit !== undefined && + element.ownerDocument.activeElement === element && + Object.is(previous.rowIdentity, rowIdentity) && + previous.pathKey === pathKey && + previous.presentationIdentity === presentationIdentity; + scalarEditState.set(element, { + edit: preserveEdit ? previous.edit : undefined, + pathKey, + presentationIdentity, + rowIdentity, + }); +} + +export function beginScalarEdit( + target: HTMLInputElement, + initialBranch: ScalarValueBranch | undefined, +): ScalarEditHint { + const state = scalarEditState.get(target); + if (!state) { + return { branch: initialBranch }; + } + state.edit ??= { branch: initialBranch }; + return state.edit; +} + +export function scalarEditHintForInput( + target: HTMLInputElement, + initialBranch: ScalarValueBranch | undefined, +): ScalarEditHint { + return scalarEditState.get(target)?.edit ?? { branch: initialBranch }; +} + +export function finishScalarEdit(target: HTMLInputElement): void { + const state = scalarEditState.get(target); + if (state) { + state.edit = undefined; + } +} + +export function finishScalarEditFromEvent(event: Event): void { + if (event.currentTarget instanceof HTMLInputElement) { + finishScalarEdit(event.currentTarget); + } +} diff --git a/ui/src/components/config-form.shared.ts b/ui/src/components/config-form.shared.ts index 2eea8b2cf6f3..4687462881b9 100644 --- a/ui/src/components/config-form.shared.ts +++ b/ui/src/components/config-form.shared.ts @@ -45,6 +45,29 @@ export function schemaType(schema: JsonSchema): string | undefined { return schema.type; } +export function schemaMayAcceptString(schema: JsonSchema): boolean { + const declaredTypes = Array.isArray(schema.type) ? schema.type : schema.type ? [schema.type] : []; + if (declaredTypes.length > 0 && !declaredTypes.includes("string")) { + return false; + } + if (schema.const !== undefined && typeof schema.const !== "string") { + return false; + } + if (schema.enum && !schema.enum.some((entry) => typeof entry === "string")) { + return false; + } + if (schema.allOf && !schema.allOf.every(schemaMayAcceptString)) { + return false; + } + if (schema.anyOf && !schema.anyOf.some(schemaMayAcceptString)) { + return false; + } + if (schema.oneOf && !schema.oneOf.some(schemaMayAcceptString)) { + return false; + } + return true; +} + export function configFieldId(path: Array, suffix: string): string { const key = path.length === 0 diff --git a/ui/src/e2e/config-safe-write.e2e.test.ts b/ui/src/e2e/config-safe-write.e2e.test.ts index 87bc11785e6a..6270ae3827b1 100644 --- a/ui/src/e2e/config-safe-write.e2e.test.ts +++ b/ui/src/e2e/config-safe-write.e2e.test.ts @@ -1,5 +1,5 @@ // Control UI browser proof covers the config snapshot and guarded-write lifecycle. -import { mkdir } from "node:fs/promises"; +import { mkdir, writeFile } from "node:fs/promises"; import path from "node:path"; import type { Locator, Page } from "playwright"; import { expect, it } from "vitest"; @@ -59,6 +59,22 @@ function configSchemaResponse() { tools: { type: "object", title: "Tools", + properties: { + elevated: { + type: "object", + properties: { + allowFrom: { + type: "object", + additionalProperties: { + type: "array", + items: { + anyOf: [{ type: "string", pattern: "^[0-9]+$" }, { type: "number" }], + }, + }, + }, + }, + }, + }, additionalProperties: true, }, }, @@ -445,4 +461,73 @@ suite.define(() => { }, ); }); + + it("preserves untouched 64-bit identifier strings during an unrelated form save", async () => { + await suite.withPage( + { + colorScheme: "dark", + locale: "en-US", + recordVideo: captureUiProofEnabled + ? { dir: uiProofArtifactDir, size: { height: 1000, width: 1440 } } + : undefined, + serviceWorkers: "block", + viewport: { height: 1000, width: 1440 }, + }, + async ({ page }) => { + const identifier = "1048113311314608148"; + const initialConfig = { + laboratory: { endpoint: "before-save", retryBudget: 2 }, + tools: { elevated: { allowFrom: { discord: [identifier, 42] } } }, + }; + const gateway = await installMockGateway(page, { + methodResponses: { + "config.get": configResponse(initialConfig, "id-snapshot-1"), + "config.schema": configSchemaResponse(), + }, + }); + + expect( + ( + await page.goto(`${suite.server.baseUrl}settings/advanced?section=laboratory`) + )?.status(), + ).toBe(200); + const endpoint = page.getByRole("textbox", { name: "Endpoint", exact: true }); + await expect.poll(() => endpoint.inputValue()).toBe("before-save"); + await capture(page, "08-id-before-unrelated-save.png"); + + await gateway.deferNext("config.set"); + await endpoint.fill("after-save"); + const save = mutationParams(await gateway.waitForRequest("config.set")); + const submitted = JSON.parse(String(save.raw)) as typeof initialConfig; + expect(save.baseHash).toBe("id-snapshot-1"); + expect(String(save.raw)).toContain(`"${identifier}"`); + expect(String(save.raw)).not.toContain(String(Number(identifier))); + expect(submitted).toEqual({ + laboratory: { endpoint: "after-save", retryBudget: 2 }, + tools: { elevated: { allowFrom: { discord: [identifier, 42] } } }, + }); + expect(submitted.tools.elevated.allowFrom.discord[0]).toBe(identifier); + expect(typeof submitted.tools.elevated.allowFrom.discord[0]).toBe("string"); + + if (captureUiProofEnabled) { + await mkdir(uiProofArtifactDir, { recursive: true }); + await writeFile( + path.join(uiProofArtifactDir, "09-id-config-set-payload.json"), + `${JSON.stringify({ before: initialConfig, submitted }, null, 2)}\n`, + ); + } + await gateway.resolveDeferred("config.set"); + const saveIndicator = page.locator("openclaw-settings-save-indicator"); + await expect.poll(() => saveIndicator.textContent()).toContain("Saved"); + + await page.reload(); + await expect.poll(() => endpoint.inputValue()).toBe("after-save"); + await page.getByRole("button", { name: "Raw", exact: true }).click(); + const rawEditor = page.locator(".config-raw-field textarea"); + await rawEditor.waitFor(); + await expect.poll(() => rawEditor.inputValue()).toContain(`"${identifier}"`); + await capture(page, "10-id-after-unrelated-save.png"); + }, + ); + }); }); diff --git a/ui/src/e2e/control-ui-auth-transports.e2e.test.ts b/ui/src/e2e/control-ui-auth-transports.e2e.test.ts index 6e2bea517ac6..3168ce199147 100644 --- a/ui/src/e2e/control-ui-auth-transports.e2e.test.ts +++ b/ui/src/e2e/control-ui-auth-transports.e2e.test.ts @@ -1,5 +1,6 @@ // Control UI tests prove trusted-proxy and browser-origin auth through real transports. -import { mkdir, writeFile } from "node:fs/promises"; +import { createHash } from "node:crypto"; +import { mkdir, readFile, writeFile } from "node:fs/promises"; import { createServer, type IncomingMessage } from "node:http"; import net from "node:net"; import path from "node:path"; @@ -34,6 +35,9 @@ const artifactDir = path.resolve( ); const viewport = { height: 900, width: 1280 }; const trustedProxyUser = "qa-operator"; +const configProofIdentifier = "9223372036854775807"; +const configProofPrefixBefore = "proof-before"; +const configProofPrefixAfter = "proof-after"; const controlUiSettleTimeoutMs = 60_000; const originProxyHeaderBlocklist = new Set([ "connection", @@ -66,6 +70,7 @@ type ProxyConnectionEvidence = { browserOrigin: string | null; gatewayResult?: GatewayResultEvidence; identityInjected: boolean; + requestMethods: string[]; requiredHeaderInjected: boolean; route: ProxyRoute; upstreamHandshakeStatus?: number; @@ -81,6 +86,7 @@ type RealTransportProxy = { type RealGateway = { cleanup: () => Promise; + httpUrl: string; port: number; server: GatewayServer; state: OpenClawTestState; @@ -165,6 +171,7 @@ function sanitizeProxyEvidence(evidence: ProxyConnectionEvidence) { browserOriginPresent: Boolean(evidence.browserOrigin), gatewayResult: evidence.gatewayResult, identityInjected: evidence.identityInjected, + requestMethods: evidence.requestMethods, requiredHeaderInjected: evidence.requiredHeaderInjected, route: evidence.route, upstreamHandshakeStatus: evidence.upstreamHandshakeStatus, @@ -199,6 +206,10 @@ function startProxyConnection( const frame = parseJsonFrame(data); if (frame) { connectRequestId = captureBrowserConnect(evidence, frame) ?? connectRequestId; + const method = frame.type === "req" ? stringValue(frame.method) : null; + if (method && method !== "connect") { + evidence.requestMethods.push(method); + } } if (upstream.readyState === WebSocket.OPEN) { upstream.send(data, { binary: isBinary }); @@ -280,6 +291,7 @@ async function startRealTransportProxy(gatewayUrl: string): Promise { async function startRealGateway(allowedOrigin: string): Promise { const port = await getFreePort(); + const httpUrl = `http://127.0.0.1:${port}/`; const state = await createOpenClawTestState({ label: "control-ui-auth-transports", layout: "home", @@ -398,20 +411,33 @@ async function startRealGateway(allowedOrigin: string): Promise { allowUsers: [trustedProxyUser], deviceAutoApprove: { enabled: true, - scopes: ["operator.approvals", "operator.questions", "operator.read", "operator.write"], + scopes: [ + "operator.admin", + "operator.approvals", + "operator.questions", + "operator.read", + "operator.write", + ], }, requiredHeaders: ["x-forwarded-proto"], userHeader: "x-forwarded-user", }; await state.writeConfig({ + messages: { responsePrefix: configProofPrefixBefore }, + tools: { + elevated: { + allowFrom: { discord: [configProofIdentifier] }, + }, + }, gateway: { auth: { mode: "trusted-proxy", trustedProxy, }, controlUi: { - allowedOrigins: [allowedOrigin], - enabled: false, + allowedOrigins: [allowedOrigin, new URL(httpUrl).origin], + enabled: true, + root: path.resolve("dist/control-ui"), }, port, trustedProxies: ["127.0.0.1", "::1"], @@ -426,7 +452,7 @@ async function startRealGateway(allowedOrigin: string): Promise { trustedProxy, }, bind: "loopback", - controlUiEnabled: false, + controlUiEnabled: true, sidecarStartup: "defer", }); return { @@ -434,6 +460,7 @@ async function startRealGateway(allowedOrigin: string): Promise { await server.close({ reason: "control ui auth transports test cleanup" }); await state.cleanup(); }, + httpUrl, port, server, state, @@ -478,8 +505,8 @@ async function createBrowserPage( waitUntil: "domcontentloaded", }); expect(response?.status()).toBe(200); - // Source-served UI startup shares CI shard CPU. Bound navigation and the - // first rendered interaction separately; transport assertions stay narrow. + // Browser startup shares CI shard CPU. Bound navigation and the first + // rendered interaction separately; transport assertions stay narrow. const confirmation = page.locator("openclaw-gateway-url-confirmation"); await confirmation.waitFor({ timeout: controlUiSettleTimeoutMs }); expect(await confirmation.textContent()).toContain(gatewayUrl); @@ -524,6 +551,42 @@ async function captureChromiumScreenshot(page: Page, fileName: string): Promise< } } +async function verifyGatewayServedControlUiBundle(httpUrl: string): Promise<{ + assetPath: string; + assetSha256: string; +}> { + const distRoot = path.resolve("dist/control-ui"); + const builtIndex = await readFile(path.join(distRoot, "index.html"), "utf8"); + const assetPath = builtIndex.match(/]+src="\.\/(assets\/[^"]+\.js)"/u)?.[1]; + if (!assetPath) { + throw new Error("built Control UI index has no JavaScript asset path"); + } + const servedIndexResponse = await fetch(httpUrl); + expect(servedIndexResponse.status).toBe(200); + expect(await servedIndexResponse.text()).toContain(`src="/${assetPath}"`); + const servedAssetResponse = await fetch(new URL(assetPath, httpUrl)); + expect(servedAssetResponse.status).toBe(200); + const servedAsset = Buffer.from(await servedAssetResponse.arrayBuffer()); + const builtAsset = await readFile(path.join(distRoot, assetPath)); + const hash = (value: Buffer) => createHash("sha256").update(value).digest("hex"); + const assetSha256 = hash(builtAsset); + expect(hash(servedAsset)).toBe(assetSha256); + return { assetPath, assetSha256 }; +} + +async function readConfigProofSnapshot(): Promise<{ identifier: unknown; prefix: string | null }> { + const config = asNullableRecord(JSON.parse(await readFile(gateway.state.configPath, "utf8"))); + const messages = asNullableRecord(config?.messages); + const tools = asNullableRecord(config?.tools); + const elevated = asNullableRecord(tools?.elevated); + const allowFrom = asNullableRecord(elevated?.allowFrom); + const discord = Array.isArray(allowFrom?.discord) ? allowFrom.discord : []; + return { + identifier: discord[0], + prefix: stringValue(messages?.responsePrefix), + }; +} + async function waitForConnectionEvidence( predicate: (entry: ProxyConnectionEvidence) => boolean, evidenceStartIndex: number, @@ -563,6 +626,7 @@ async function isPortClosed(host: string, port: number): Promise { describeControlUiE2e("Control UI real auth transports E2E", () => { beforeAll(async () => { + console.info("[real-config-id-proof] setup-start"); if (!chromiumAvailable) { throw new Error( `Playwright Chromium is not installed or cannot start at ${chromiumExecutablePath}.`, @@ -576,6 +640,7 @@ describeControlUiE2e("Control UI real auth transports E2E", () => { gateway = await startRealGateway(new URL(allowedUi.baseUrl).origin); proxy = await startRealTransportProxy(gateway.url); browser = await chromium.launch({ executablePath: chromiumExecutablePath }); + console.info("[real-config-id-proof] setup-ready"); }, 120_000); afterAll(async () => { @@ -614,6 +679,91 @@ describeControlUiE2e("Control UI real auth transports E2E", () => { openContexts.clear(); }); + it("preserves a 64-bit identifier through a real Gateway form save", async () => { + const servedBundle = await verifyGatewayServedControlUiBundle(gateway.httpUrl); + const connected = await createBrowserPage(gateway.httpUrl, proxy.trustedUrl); + await connected.page + .locator("openclaw-app-shell") + .waitFor({ timeout: controlUiSettleTimeoutMs }); + const servedAssetLoaded = await connected.page.evaluate( + (assetPath) => + performance + .getEntriesByType("resource") + .some((entry) => new URL(entry.name).pathname.endsWith(`/${assetPath}`)), + servedBundle.assetPath, + ); + expect(servedAssetLoaded).toBe(true); + + const rawSettingsUrl = new URL("settings/advanced", gateway.httpUrl); + rawSettingsUrl.searchParams.set("section", "env"); + expect((await connected.page.goto(rawSettingsUrl.toString()))?.status()).toBe(200); + await connected.page.getByRole("button", { name: "Raw", exact: true }).click(); + const rawEditorBefore = connected.page.locator(".config-raw-field textarea"); + await rawEditorBefore.waitFor(); + await expect.poll(() => rawEditorBefore.inputValue()).toContain(`"${configProofIdentifier}"`); + await expect.poll(() => rawEditorBefore.inputValue()).toContain(configProofPrefixBefore); + await rawEditorBefore.scrollIntoViewIfNeeded(); + await captureChromiumScreenshot(connected.page, "01-real-config-id-before.png"); + + const settingsUrl = new URL("settings/communications", gateway.httpUrl); + settingsUrl.searchParams.set("section", "messages"); + expect((await connected.page.goto(settingsUrl.toString()))?.status()).toBe(200); + const prefix = connected.page.getByRole("textbox", { + name: "Outbound Response Prefix", + exact: true, + }); + await expect.poll(() => prefix.inputValue()).toBe(configProofPrefixBefore); + const configSetCount = () => + proxy.evidence + .slice(connected.evidenceStartIndex) + .flatMap((entry) => entry.requestMethods) + .filter((method) => method === "config.set").length; + const configSetCountBefore = configSetCount(); + await prefix.fill(configProofPrefixAfter); + + await expect.poll(configSetCount, { timeout: 15_000 }).toBeGreaterThan(configSetCountBefore); + await expect + .poll(async () => (await readConfigProofSnapshot()).prefix) + .toBe(configProofPrefixAfter); + const persisted = await readConfigProofSnapshot(); + expect(persisted.identifier).toBe(configProofIdentifier); + expect(typeof persisted.identifier).toBe("string"); + + await connected.page.reload({ waitUntil: "domcontentloaded" }); + await connected.page + .locator("openclaw-app-shell") + .waitFor({ timeout: controlUiSettleTimeoutMs }); + expect((await connected.page.goto(rawSettingsUrl.toString()))?.status()).toBe(200); + await connected.page.getByRole("button", { name: "Raw", exact: true }).click(); + const rawEditor = connected.page.locator(".config-raw-field textarea"); + await rawEditor.waitFor(); + await expect.poll(() => rawEditor.inputValue()).toContain(`"${configProofIdentifier}"`); + await expect.poll(() => rawEditor.inputValue()).toContain(configProofPrefixAfter); + await rawEditor.scrollIntoViewIfNeeded(); + + const proof = { + configSetRequests: configSetCount() - configSetCountBefore, + identifierMatches: persisted.identifier === configProofIdentifier, + identifierType: typeof persisted.identifier, + method: "config.set", + persistedPrefix: persisted.prefix, + rawReadbackQuoted: true, + servedAssetLoaded, + servedAssetPath: servedBundle.assetPath, + servedAssetSha256: servedBundle.assetSha256, + uiSource: "gateway-dist-control-ui", + }; + await writeFile( + path.join(artifactDir, "real-gateway-config-id-proof.json"), + `${JSON.stringify(proof, null, 2)}\n`, + "utf8", + ); + console.info(`[real-config-id-proof] ${JSON.stringify(proof)}`); + await captureChromiumScreenshot(connected.page, "02-real-config-id-after.png"); + expect(connected.errors).toEqual([]); + await closeConnectedContext(connected.context); + }); + it("connects through the trusted path and rejects the untrusted proxy path", async () => { // A connected shell starts bootstrap RPCs that can outlive context teardown. // Keep it last so those requests cannot starve the next browser interaction. diff --git a/ui/src/lib/config/config-draft-model.test.ts b/ui/src/lib/config/config-draft-model.test.ts index 53d82ad412c4..96fab4abb5fc 100644 --- a/ui/src/lib/config/config-draft-model.test.ts +++ b/ui/src/lib/config/config-draft-model.test.ts @@ -142,6 +142,20 @@ describe("config draft model", () => { fractionalInteger: { type: "integer" }, unionRadix: { anyOf: [{ type: "integer" }, { type: "string" }] }, unionScientific: { anyOf: [{ type: "integer" }, { type: "string" }] }, + unionDigits: { + oneOf: [{ type: "integer" }, { type: "string", pattern: "^[0-9]+$" }], + }, + unionEnum: { + anyOf: [ + { type: "number", const: 60 }, + { type: "string", enum: ["60"] }, + ], + }, + unionConstOnly: { anyOf: [{ const: "60" }, { type: "number" }] }, + unionEnumOnly: { oneOf: [{ enum: ["60"] }, { type: "number" }] }, + unionBooleanConstOnly: { + anyOf: [{ const: "true" }, { type: "boolean" }], + }, }, }, uiHints: {}, @@ -165,6 +179,11 @@ describe("config draft model", () => { runtimeConfig.patchForm(["fractionalInteger"], "42.5"); runtimeConfig.patchForm(["unionRadix"], "0o17"); runtimeConfig.patchForm(["unionScientific"], "1e5"); + runtimeConfig.patchForm(["unionDigits"], "00123"); + runtimeConfig.patchForm(["unionEnum"], "60"); + runtimeConfig.patchForm(["unionConstOnly"], "60"); + runtimeConfig.patchForm(["unionEnumOnly"], "60"); + runtimeConfig.patchForm(["unionBooleanConstOnly"], "true"); await expect(runtimeConfig.save()).resolves.toBe(true); const submission = submitted.find((entry) => entry.method === "config.set"); @@ -180,7 +199,82 @@ describe("config draft model", () => { decimal: 0.5, fractionalInteger: "42.5", unionRadix: "0o17", - unionScientific: 100_000, + // String-capable unions keep the text input; the Gateway owns constraints. + unionScientific: "1e5", + unionDigits: "00123", + unionEnum: "60", + unionConstOnly: "60", + unionEnumOnly: "60", + unionBooleanConstOnly: "true", + }); + runtimeConfig.dispose(); + }); + + it("preserves 64-bit id strings through the form submit roundtrip", async () => { + const submitted: Array<{ method: string; params: unknown }> = []; + const request = vi.fn(async (method: string, params?: unknown) => { + if (method === "config.get") { + return { + config: { + allowFrom: { discord: ["1048113311314608148", 42] }, + label: "before", + }, + hash: "hash-1", + valid: true, + issues: [], + }; + } + if (method === "config.schema") { + return { + schema: { + type: "object", + properties: { + allowFrom: { + type: "object", + additionalProperties: { + type: "array", + items: { + oneOf: [ + { + type: "string", + allOf: [{ pattern: "^[0-9]+$" }], + not: { const: "never" }, + }, + { type: "number" }, + ], + }, + }, + }, + bigInteger: { type: "integer" }, + label: { type: "string" }, + }, + }, + uiHints: {}, + }; + } + submitted.push({ method, params }); + return { hash: "hash-2" }; + }); + const client = { request } as unknown as GatewayBrowserClient; + const { gateway } = createGatewayHarness(client); + const runtimeConfig = createRuntimeConfigCapability(gateway); + + await Promise.all([runtimeConfig.ensureLoaded(), runtimeConfig.ensureSchemaLoaded()]); + // Only the unrelated label is edited; the untouched allowFrom entry must + // come back byte-identical instead of collapsing to Number precision. + runtimeConfig.patchForm(["label"], "after"); + runtimeConfig.patchForm(["bigInteger"], "10481133113146081487"); + + await expect(runtimeConfig.save()).resolves.toBe(true); + const submission = submitted.find((entry) => entry.method === "config.set"); + const raw = (submission?.params as { raw?: unknown } | undefined)?.raw; + expect(typeof raw).toBe("string"); + expect(JSON.parse(raw as string)).toEqual({ + allowFrom: { discord: ["1048113311314608148", 42] }, + // Beyond 2^53 an unsafe integer parse must not happen even for pure + // integer fields; the string is kept for the gateway to reject loudly. + bigInteger: "10481133113146081487", + label: "after", }); runtimeConfig.dispose(); }); diff --git a/ui/src/lib/config/config-draft-model.ts b/ui/src/lib/config/config-draft-model.ts index 22eb2fdd0f39..c9a3d927a5eb 100644 --- a/ui/src/lib/config/config-draft-model.ts +++ b/ui/src/lib/config/config-draft-model.ts @@ -5,7 +5,11 @@ import { import { GatewayRequestError } from "../../api/gateway.ts"; import type { ConfigSnapshot } from "../../api/types.ts"; import { coerceConfigFormNumberString } from "../../components/config-form.numeric.ts"; -import { schemaType, type JsonSchema } from "../../components/config-form.shared.ts"; +import { + schemaMayAcceptString, + schemaType, + type JsonSchema, +} from "../../components/config-form.shared.ts"; import { t } from "../../i18n/index.ts"; import { cloneConfigObject, @@ -208,6 +212,12 @@ function coerceFormValues(value: unknown, schema: JsonSchema): unknown { return variant ? coerceFormValues(value, variant) : value; } if (typeof value === "string") { + // Editors commit branch-validated types (including boolean literals), + // and loaded values already passed Gateway validation. Preserve strings + // instead of guessing again here. + if (variants.some(schemaMayAcceptString)) { + return value; + } for (const variant of variants) { const variantType = schemaType(variant); if (variantType === "number" || variantType === "integer") {