From bfa359e8f9bb45fac32bb9f86bf00ee6ea293999 Mon Sep 17 00:00:00 2001 From: syn Date: Wed, 27 May 2026 14:00:11 -0500 Subject: [PATCH 1/5] feat(kiloclaw): validate openclaw config file saves --- .specs/kiloclaw-controller.md | 12 + .../(app)/claw/components/FileEditorPane.tsx | 144 ++++++++--- .../app/(app)/claw/components/SettingsTab.tsx | 3 + .../claw/components/WorkspaceFileEditor.tsx | 19 +- .../(app)/claw/components/changelog-data.ts | 7 + .../KiloclawInstances/AdminFileEditor.tsx | 41 +++- .../KiloclawInstanceDetail.tsx | 8 +- apps/web/src/hooks/useKiloClaw.ts | 3 +- apps/web/src/hooks/useOrgKiloClaw.ts | 3 +- .../lib/kiloclaw/kiloclaw-internal-client.ts | 25 ++ .../admin-kiloclaw-instances-router.ts | 16 ++ apps/web/src/routers/kiloclaw-router.ts | 16 ++ .../organization-kiloclaw-router.test.ts | 44 ++++ .../organization-kiloclaw-router.ts | 16 ++ .../src/endpoint-capabilities.test.ts | 4 + .../controller/src/endpoint-capabilities.ts | 1 + .../src/openclaw-config-validation.test.ts | 142 +++++++++++ .../src/openclaw-config-validation.ts | 230 ++++++++++++++++++ .../controller/src/routes/files.test.ts | 133 ++++++++++ .../kiloclaw/controller/src/routes/files.ts | 92 ++++++- .../controller-entrypoint-smoke-test.sh | 14 ++ .../gateway-controller-types.test.ts | 19 +- .../gateway-controller-types.ts | 20 ++ .../kiloclaw-instance/gateway.test.ts | 42 ++++ .../kiloclaw-instance/gateway.ts | 31 ++- .../kiloclaw-instance/index.ts | 10 + .../routes/platform-sanitize-error.test.ts | 81 ++++++ services/kiloclaw/src/routes/platform.ts | 42 ++++ 28 files changed, 1165 insertions(+), 53 deletions(-) create mode 100644 services/kiloclaw/controller/src/openclaw-config-validation.test.ts create mode 100644 services/kiloclaw/controller/src/openclaw-config-validation.ts diff --git a/.specs/kiloclaw-controller.md b/.specs/kiloclaw-controller.md index 3306718ac9..7d74151173 100644 --- a/.specs/kiloclaw-controller.md +++ b/.specs/kiloclaw-controller.md @@ -730,6 +730,18 @@ running. When forwarding, it MUST authenticate to gog with | GET | `/_kilo/files/read` | Read a safe file path | | POST | `/_kilo/files/import-openclaw-workspace` | Import OpenClaw workspace files | | POST | `/_kilo/files/write` | Write a safe file path | +| POST | `/_kilo/files/write-openclaw-config` | Validate and write `openclaw.json`, with explicit invalid override support | + +##### Validation-aware `openclaw.json` file writes + +1. `POST /_kilo/files/write-openclaw-config` MUST provide the validation-aware save flow for `openclaw.json`; clients MUST NOT use optional fields on generic `/_kilo/files/write` to infer validation behavior. +2. For a normal validation-aware save, the controller MUST evaluate the submitted candidate using the installed OpenClaw config-validation behavior before committing it. +3. When the candidate is invalid or validation cannot complete, the controller MUST leave `openclaw.json` unchanged and MUST return a bounded structured warning result suitable for an authenticated client to display. +4. The controller MAY accept an explicit invalid-write override after a warning. An override MUST remain subject to normal safe-path and ETag concurrency checks and MUST NOT be inferred from an ordinary save request. +5. Validation-warning and override writes MUST NOT expose config contents, secrets, environment values, staging paths, or unrestricted subprocess output in responses or logs. +6. Validation-aware writes MUST remain usable when the gateway process is unavailable after controller routes are registered. +7. Controllers implementing this behavior MUST advertise `files.write-openclaw-config`; clients MUST NOT infer this behavior solely from controller CalVer. +8. This validation reports OpenClaw configuration validity. It MUST NOT be represented as proof that runtime SecretRefs or optional environment substitutions resolve successfully. #### Doctor (bearer token) diff --git a/apps/web/src/app/(app)/claw/components/FileEditorPane.tsx b/apps/web/src/app/(app)/claw/components/FileEditorPane.tsx index d39aa003fb..a6d38ffeb9 100644 --- a/apps/web/src/app/(app)/claw/components/FileEditorPane.tsx +++ b/apps/web/src/app/(app)/claw/components/FileEditorPane.tsx @@ -6,6 +6,20 @@ import { Loader2 } from 'lucide-react'; import { toast } from 'sonner'; import { Button } from '@/components/ui/button'; import { Alert, AlertDescription } from '@/components/ui/alert'; +import { + AlertDialog, + AlertDialogAction, + AlertDialogCancel, + AlertDialogContent, + AlertDialogDescription, + AlertDialogFooter, + AlertDialogHeader, + AlertDialogTitle, +} from '@/components/ui/alert-dialog'; +import type { + FileWriteResponse, + OpenclawFileWriteValidation, +} from '@/lib/kiloclaw/kiloclaw-internal-client'; const Editor = lazy>(() => import('@monaco-editor/react')); const DiffEditor = lazy>(() => @@ -61,15 +75,21 @@ export interface FileEditorPaneProps { error: { message: string } | null; refetch: () => void; onSave: ( - args: { path: string; content: string; etag?: string }, + args: { + path: string; + content: string; + etag?: string; + openclawValidation?: OpenclawFileWriteValidation; + }, callbacks: { - onSuccess: (result: { etag: string }) => void; + onSuccess: (result: FileWriteResponse) => void; onError: (err: FileSaveError) => void; } ) => void; isSaving: boolean; onDirtyChange?: (dirty: boolean) => void; validateBeforeSave?: (filePath: string, content: string) => boolean; + enableOpenclawValidation?: boolean; } export function FileEditorPane({ @@ -82,8 +102,13 @@ export function FileEditorPane({ isSaving, onDirtyChange, validateBeforeSave, + enableOpenclawValidation = false, }: FileEditorPaneProps) { const [showDiff, setShowDiff] = useState(false); + const [pendingValidationWarning, setPendingValidationWarning] = useState< + | (Extract & { content: string }) + | null + >(null); // savedContentRef holds the last successfully saved content, used as fallback // until the query refetches to avoid flashing stale content after save. @@ -98,6 +123,7 @@ export function FileEditorPane({ prevFilePathRef.current = filePath; setEditedContent(null); setShowDiff(false); + setPendingValidationWarning(null); etagRef.current = undefined; savedContentRef.current = null; } @@ -132,6 +158,39 @@ export function FileEditorPane({ [serverContent] ); + const submitSave = useCallback( + (content: string, openclawValidation?: OpenclawFileWriteValidation) => { + onSave( + { path: filePath, content, etag: etagRef.current, openclawValidation }, + { + onSuccess: result => { + if ('outcome' in result) { + setPendingValidationWarning({ ...result, content }); + return; + } + etagRef.current = result.etag; + savedContentRef.current = content; + setEditedContent(null); + setPendingValidationWarning(null); + toast.success(`Saved ${filePath}`); + }, + onError: err => { + if (err.data?.code === 'CONFLICT' && err.data?.upstreamCode === 'file_etag_conflict') { + refetch(); + setShowDiff(true); + toast.error( + 'File was modified externally — your edits are preserved, review the diff' + ); + } else { + toast.error(err.message); + } + }, + } + ); + }, + [filePath, onSave, refetch] + ); + if (isLoading) { return ; } @@ -226,34 +285,11 @@ export function FileEditorPane({ if (validateBeforeSave && !validateBeforeSave(filePath, currentValue)) { return; } - onSave( - { path: filePath, content: currentValue, etag: etagRef.current }, - { - onSuccess: result => { - etagRef.current = result.etag; - // Optimistically update serverContent so we don't flash stale content - // while the invalidated readFile query refetches. - savedContentRef.current = currentValue; - setEditedContent(null); - toast.success(`Saved ${filePath}`); - }, - onError: err => { - if ( - err.data?.code === 'CONFLICT' && - err.data?.upstreamCode === 'file_etag_conflict' - ) { - // Preserve the user's edits and show diff so they can compare - // their version against the server's updated content. - refetch(); - setShowDiff(true); - toast.error( - 'File was modified externally — your edits are preserved, review the diff' - ); - } else { - toast.error(err.message); - } - }, - } + submitSave( + currentValue, + enableOpenclawValidation && filePath === 'openclaw.json' + ? 'warn-before-write' + : undefined ); }} > @@ -268,6 +304,54 @@ export function FileEditorPane({ + + { + if (!open && !isSaving) setPendingValidationWarning(null); + }} + > + + + + {pendingValidationWarning?.reason === 'invalid' + ? 'OpenClaw configuration is invalid' + : 'Configuration validation could not run'} + + + {pendingValidationWarning?.reason === 'invalid' + ? 'OpenClaw may reject this file or restore the previous configuration during reload or startup.' + : 'Save without validation only if you understand that OpenClaw may reject or restore this file.'} + + + {pendingValidationWarning && ( +
+
    + {pendingValidationWarning.issues.map((issue, index) => ( +
  • + {issue.path &&
    {issue.path}
    } +
    {issue.message}
    +
    • + ))} +
+
+ )} + + Keep editing + { + if (pendingValidationWarning) { + submitSave(pendingValidationWarning.content, 'allow-invalid'); + } + }} + > + {isSaving ? 'Saving...' : 'Save anyway'} + + + + ); } diff --git a/apps/web/src/app/(app)/claw/components/SettingsTab.tsx b/apps/web/src/app/(app)/claw/components/SettingsTab.tsx index 45a8c44857..c67210ebf5 100644 --- a/apps/web/src/app/(app)/claw/components/SettingsTab.tsx +++ b/apps/web/src/app/(app)/claw/components/SettingsTab.tsx @@ -2003,6 +2003,8 @@ export function SettingsTab({ cleanVersion(controllerVersion?.version), OPENCLAW_IMPORT_UI_MIN_CONTROLLER_VERSION ); + const supportsOpenclawSaveValidation = + controllerVersion?.capabilities?.includes('files.write-openclaw-config') === true; // Fail OPEN: hide the interests editor only when the controller // version is positively parsed as too old, OR the worker reports an // explicit `version: null` (its positive old-controller signal for a @@ -2605,6 +2607,7 @@ export function SettingsTab({ enabled={isRunning} mutations={mutations} onOpenChange={setEditConfigOpen} + enableOpenclawValidation={supportsOpenclawSaveValidation} /> )} diff --git a/apps/web/src/app/(app)/claw/components/WorkspaceFileEditor.tsx b/apps/web/src/app/(app)/claw/components/WorkspaceFileEditor.tsx index c8295f09c3..abbf0b2764 100644 --- a/apps/web/src/app/(app)/claw/components/WorkspaceFileEditor.tsx +++ b/apps/web/src/app/(app)/claw/components/WorkspaceFileEditor.tsx @@ -3,6 +3,10 @@ import { useCallback } from 'react'; import { useClawFileTree, useClawReadFile } from '../hooks/useClawHooks'; import type { useKiloClawMutations } from '@/hooks/useKiloClaw'; +import type { + FileWriteResponse, + OpenclawFileWriteValidation, +} from '@/lib/kiloclaw/kiloclaw-internal-client'; import { FileEditorShell } from './FileEditorShell'; import { FileEditorPane, type FileSaveError } from './FileEditorPane'; import { validateOpenclawJsonForSave } from './validateOpenclawJson'; @@ -14,19 +18,26 @@ function UserFileEditorPane({ enabled, mutations, onDirtyChange, + enableOpenclawValidation, }: { filePath: string; enabled: boolean; mutations: ClawMutations; onDirtyChange: (dirty: boolean) => void; + enableOpenclawValidation: boolean; }) { const { data, isLoading, error, refetch } = useClawReadFile(filePath, enabled); const handleSave = useCallback( ( - args: { path: string; content: string; etag?: string }, + args: { + path: string; + content: string; + etag?: string; + openclawValidation?: OpenclawFileWriteValidation; + }, callbacks: { - onSuccess: (result: { etag: string }) => void; + onSuccess: (result: FileWriteResponse) => void; onError: (err: FileSaveError) => void; } ) => { @@ -47,6 +58,7 @@ function UserFileEditorPane({ isSaving={mutations.writeFile.isPending} onDirtyChange={onDirtyChange} validateBeforeSave={validateOpenclawJsonForSave} + enableOpenclawValidation={enableOpenclawValidation} /> ); } @@ -55,10 +67,12 @@ export function WorkspaceFileEditor({ enabled, mutations, onOpenChange, + enableOpenclawValidation, }: { enabled: boolean; mutations: ClawMutations; onOpenChange: (open: boolean) => void; + enableOpenclawValidation: boolean; }) { const { data: tree, isLoading, error, refetch } = useClawFileTree(enabled); @@ -76,6 +90,7 @@ export function WorkspaceFileEditor({ enabled={enabled} mutations={mutations} onDirtyChange={onDirtyChange} + enableOpenclawValidation={enableOpenclawValidation} /> )} /> diff --git a/apps/web/src/app/(app)/claw/components/changelog-data.ts b/apps/web/src/app/(app)/claw/components/changelog-data.ts index f5d98c715f..449511ace4 100644 --- a/apps/web/src/app/(app)/claw/components/changelog-data.ts +++ b/apps/web/src/app/(app)/claw/components/changelog-data.ts @@ -10,6 +10,13 @@ export type ChangelogEntry = { // Newest entries first. Developers add new entries to the top of this array. export const CHANGELOG_ENTRIES: ChangelogEntry[] = [ + { + date: '2026-05-27', + description: + 'Saving openclaw.json from Edit Files now checks the installed OpenClaw configuration rules first. If validation fails, your edits are preserved and you can review the warning before choosing to save anyway.', + category: 'feature', + deployHint: 'upgrade_required', + }, { date: '2026-05-20', description: diff --git a/apps/web/src/app/admin/components/KiloclawInstances/AdminFileEditor.tsx b/apps/web/src/app/admin/components/KiloclawInstances/AdminFileEditor.tsx index 134283be68..b4ce98e148 100644 --- a/apps/web/src/app/admin/components/KiloclawInstances/AdminFileEditor.tsx +++ b/apps/web/src/app/admin/components/KiloclawInstances/AdminFileEditor.tsx @@ -8,6 +8,10 @@ import { Button } from '@/components/ui/button'; import { FileEditorShell } from '@/app/(app)/claw/components/FileEditorShell'; import { FileEditorPane, type FileSaveError } from '@/app/(app)/claw/components/FileEditorPane'; import { validateOpenclawJsonForSave } from '@/app/(app)/claw/components/validateOpenclawJson'; +import type { + FileWriteResponse, + OpenclawFileWriteValidation, +} from '@/lib/kiloclaw/kiloclaw-internal-client'; function AdminFileEditorPaneInner({ userId, @@ -15,12 +19,14 @@ function AdminFileEditorPaneInner({ filePath, writeFileMutation, onDirtyChange, + enableOpenclawValidation, }: { userId: string; instanceId: string; filePath: string; - writeFileMutation: ReturnType>; // eslint-disable-line @typescript-eslint/no-explicit-any + writeFileMutation: ReturnType>; // eslint-disable-line @typescript-eslint/no-explicit-any onDirtyChange: (dirty: boolean) => void; + enableOpenclawValidation: boolean; }) { const trpc = useTRPC(); const { data, isLoading, error, refetch } = useQuery( @@ -32,14 +38,26 @@ function AdminFileEditorPaneInner({ const handleSave = useCallback( ( - args: { path: string; content: string; etag?: string }, + args: { + path: string; + content: string; + etag?: string; + openclawValidation?: OpenclawFileWriteValidation; + }, callbacks: { - onSuccess: (result: { etag: string }) => void; + onSuccess: (result: FileWriteResponse) => void; onError: (err: FileSaveError) => void; } ) => { writeFileMutation.mutate( - { userId, instanceId, path: args.path, content: args.content, etag: args.etag }, + { + userId, + instanceId, + path: args.path, + content: args.content, + etag: args.etag, + openclawValidation: args.openclawValidation, + }, callbacks ); }, @@ -59,11 +77,20 @@ function AdminFileEditorPaneInner({ isSaving={writeFileMutation.isPending} onDirtyChange={onDirtyChange} validateBeforeSave={validateBeforeSave} + enableOpenclawValidation={enableOpenclawValidation} /> ); } -export function AdminFileEditor({ userId, instanceId }: { userId: string; instanceId: string }) { +export function AdminFileEditor({ + userId, + instanceId, + enableOpenclawValidation, +}: { + userId: string; + instanceId: string; + enableOpenclawValidation: boolean; +}) { const trpc = useTRPC(); const queryClient = useQueryClient(); const [enabled, setEnabled] = useState(false); @@ -82,7 +109,8 @@ export function AdminFileEditor({ userId, instanceId }: { userId: string; instan const writeFileMutation = useMutation( trpc.admin.kiloclawInstances.writeFile.mutationOptions({ - onSuccess: async () => { + onSuccess: async result => { + if ('outcome' in result) return; await queryClient.invalidateQueries({ queryKey: trpc.admin.kiloclawInstances.fileTree.queryKey({ userId, instanceId }), }); @@ -117,6 +145,7 @@ export function AdminFileEditor({ userId, instanceId }: { userId: string; instan filePath={selectedPath} writeFileMutation={writeFileMutation} onDirtyChange={onDirtyChange} + enableOpenclawValidation={enableOpenclawValidation} /> )} /> diff --git a/apps/web/src/app/admin/components/KiloclawInstances/KiloclawInstanceDetail.tsx b/apps/web/src/app/admin/components/KiloclawInstances/KiloclawInstanceDetail.tsx index 2835abc879..c0acd8d0b6 100644 --- a/apps/web/src/app/admin/components/KiloclawInstances/KiloclawInstanceDetail.tsx +++ b/apps/web/src/app/admin/components/KiloclawInstances/KiloclawInstanceDetail.tsx @@ -1505,6 +1505,8 @@ export function KiloclawInstanceDetail({ instanceId }: { instanceId: string }) { cleanVersion(controllerVersionResolved?.version), '2026.5.8.1900' ); + const supportsOpenclawSaveValidation = + controllerVersionResolved?.capabilities?.includes('files.write-openclaw-config') === true; // After a restart/upgrade, poll the machine status until it returns to "running", // then invalidate controllerVersion so supportsConfigRestore reflects the new build. @@ -4536,7 +4538,11 @@ export function KiloclawInstanceDetail({ instanceId }: { instanceId: string }) { - + )} diff --git a/apps/web/src/hooks/useKiloClaw.ts b/apps/web/src/hooks/useKiloClaw.ts index 7a1cac04dc..0f69709a40 100644 --- a/apps/web/src/hooks/useKiloClaw.ts +++ b/apps/web/src/hooks/useKiloClaw.ts @@ -323,7 +323,8 @@ export function useKiloClawMutations() { ), writeFile: useMutation( trpc.kiloclaw.writeFile.mutationOptions({ - onSuccess: async () => { + onSuccess: async result => { + if ('outcome' in result) return; await queryClient.invalidateQueries({ queryKey: trpc.kiloclaw.fileTree.queryKey(), }); diff --git a/apps/web/src/hooks/useOrgKiloClaw.ts b/apps/web/src/hooks/useOrgKiloClaw.ts index beecd1e009..e521dcd5a7 100644 --- a/apps/web/src/hooks/useOrgKiloClaw.ts +++ b/apps/web/src/hooks/useOrgKiloClaw.ts @@ -393,7 +393,8 @@ export function useOrgKiloClawMutations( ); const rawWriteFile = useMutation( trpc.organizations.kiloclaw.writeFile.mutationOptions({ - onSuccess: async () => { + onSuccess: async result => { + if ('outcome' in result) return; await queryClient.invalidateQueries({ queryKey: trpc.organizations.kiloclaw.fileTree.queryKey(), }); diff --git a/apps/web/src/lib/kiloclaw/kiloclaw-internal-client.ts b/apps/web/src/lib/kiloclaw/kiloclaw-internal-client.ts index 7f0acfa978..b9664d66cf 100644 --- a/apps/web/src/lib/kiloclaw/kiloclaw-internal-client.ts +++ b/apps/web/src/lib/kiloclaw/kiloclaw-internal-client.ts @@ -76,6 +76,17 @@ export interface FileNode { children?: FileNode[]; } +export type OpenclawFileWriteValidation = 'warn-before-write' | 'allow-invalid'; + +export type FileWriteResponse = + | { etag: string } + | { + outcome: 'openclaw-validation-warning'; + valid: false; + reason: 'invalid' | 'validation-unavailable'; + issues: Array<{ path: string; message: string; allowedValues?: string[] }>; + }; + /** * Error thrown when the KiloClaw API returns a non-OK response. * Preserves the HTTP status code and response body for structured @@ -914,6 +925,20 @@ export class KiloClawInternalClient { }); } + async writeOpenclawConfigFile( + userId: string, + content: string, + etag: string | undefined, + instanceId: string | undefined, + mode: OpenclawFileWriteValidation + ): Promise { + const params = instanceId ? `?instanceId=${encodeURIComponent(instanceId)}` : ''; + return this.request(`/api/platform/files/write-openclaw-config${params}`, { + method: 'POST', + body: JSON.stringify({ userId, content, etag, mode }), + }); + } + async importOpenclawWorkspace( userId: string, files: Array<{ path: string; content: string }>, diff --git a/apps/web/src/routers/admin-kiloclaw-instances-router.ts b/apps/web/src/routers/admin-kiloclaw-instances-router.ts index 344003c1d1..8d22e09efd 100644 --- a/apps/web/src/routers/admin-kiloclaw-instances-router.ts +++ b/apps/web/src/routers/admin-kiloclaw-instances-router.ts @@ -1632,12 +1632,28 @@ export const adminKiloclawInstancesRouter = createTRPCRouter({ path: z.string().min(1), content: z.string(), etag: z.string().optional(), + openclawValidation: z.enum(['warn-before-write', 'allow-invalid']).optional(), }) ) .mutation(async ({ input }) => { try { const instance = await resolveInstance(input.userId, input.instanceId); const client = new KiloClawInternalClient(); + if (input.openclawValidation) { + if (input.path !== 'openclaw.json') { + throw new TRPCError({ + code: 'BAD_REQUEST', + message: 'OpenClaw validation is only available for openclaw.json', + }); + } + return await client.writeOpenclawConfigFile( + input.userId, + input.content, + input.etag, + workerInstanceId(instance), + input.openclawValidation + ); + } return await client.writeFile( input.userId, input.path, diff --git a/apps/web/src/routers/kiloclaw-router.ts b/apps/web/src/routers/kiloclaw-router.ts index b9e59224ef..5a18c64a44 100644 --- a/apps/web/src/routers/kiloclaw-router.ts +++ b/apps/web/src/routers/kiloclaw-router.ts @@ -4293,6 +4293,7 @@ export const kiloclawRouter = createTRPCRouter({ path: z.string().min(1), content: z.string(), etag: z.string().min(1), + openclawValidation: z.enum(['warn-before-write', 'allow-invalid']).optional(), }) ) .mutation(async ({ ctx, input }) => { @@ -4320,6 +4321,21 @@ export const kiloclawRouter = createTRPCRouter({ content = JSON.stringify(userConfig, null, 2); } + if (input.openclawValidation) { + if (input.path !== 'openclaw.json') { + throw new TRPCError({ + code: 'BAD_REQUEST', + message: 'OpenClaw validation is only available for openclaw.json', + }); + } + return await client.writeOpenclawConfigFile( + ctx.user.id, + content, + input.etag, + workerInstanceId(instance), + input.openclawValidation + ); + } return await client.writeFile( ctx.user.id, input.path, diff --git a/apps/web/src/routers/organizations/organization-kiloclaw-router.test.ts b/apps/web/src/routers/organizations/organization-kiloclaw-router.test.ts index c0f14235dc..cc99d70664 100644 --- a/apps/web/src/routers/organizations/organization-kiloclaw-router.test.ts +++ b/apps/web/src/routers/organizations/organization-kiloclaw-router.test.ts @@ -27,6 +27,7 @@ type KiloClawClientMock = { __restartGatewayProcessMock: AnyMock; __startMock: AnyMock; __stopMock: AnyMock; + __writeOpenclawConfigFileMock: AnyMock; }; type KiloClawUserClientMock = { @@ -73,6 +74,7 @@ jest.mock('@/lib/kiloclaw/kiloclaw-internal-client', () => { const restartGatewayProcessMock = jest.fn(); const startMock = jest.fn(); const stopMock = jest.fn(); + const writeOpenclawConfigFileMock = jest.fn(); return { KiloClawInternalClient: jest.fn().mockImplementation(() => ({ destroy: destroyMock, @@ -81,6 +83,7 @@ jest.mock('@/lib/kiloclaw/kiloclaw-internal-client', () => { restartGatewayProcess: restartGatewayProcessMock, start: startMock, stop: stopMock, + writeOpenclawConfigFile: writeOpenclawConfigFileMock, })), KiloClawApiError: class KiloClawApiError extends Error { statusCode: number; @@ -97,6 +100,7 @@ jest.mock('@/lib/kiloclaw/kiloclaw-internal-client', () => { __restartGatewayProcessMock: restartGatewayProcessMock, __startMock: startMock, __stopMock: stopMock, + __writeOpenclawConfigFileMock: writeOpenclawConfigFileMock, }; }); @@ -461,6 +465,46 @@ describe('organizations.kiloclaw.patchWebSearchConfig', () => { }); }); +describe('organizations.kiloclaw.writeFile validation mode', () => { + beforeEach(async () => { + await cleanupDbForTest(); + kiloclawClientMock.__writeOpenclawConfigFileMock.mockReset(); + }); + + it('normalizes openclaw.json and forwards validation-aware saves', async () => { + const user = await insertTestUser({ + google_user_email: `org-kiloclaw-write-file-${crypto.randomUUID()}@example.com`, + }); + const organization = await createOrganization('Org KiloClaw File Write Test', user.id); + const instanceId = await createActiveOrgInstance(user.id, organization.id); + kiloclawClientMock.__writeOpenclawConfigFileMock.mockResolvedValue({ + outcome: 'openclaw-validation-warning', + valid: false, + reason: 'invalid', + issues: [{ path: 'gateway.mode', message: 'Expected local' }], + }); + + const caller = await createCallerForUser(user.id); + await expect( + caller.organizations.kiloclaw.writeFile({ + organizationId: organization.id, + path: 'openclaw.json', + content: '{"gateway":{"mode":"remote"}}', + etag: 'etag-1', + openclawValidation: 'warn-before-write', + }) + ).resolves.toMatchObject({ outcome: 'openclaw-validation-warning', reason: 'invalid' }); + + expect(kiloclawClientMock.__writeOpenclawConfigFileMock).toHaveBeenCalledWith( + user.id, + '{\n "gateway": {\n "mode": "remote"\n }\n}', + 'etag-1', + instanceId, + 'warn-before-write' + ); + }); +}); + describe('organizations.kiloclaw.restartMachine pin consent gate', () => { // The pin row has an FK to kiloclaw_image_catalog.image_tag, so we // need real catalog rows for pin inserts. The restartMachine input diff --git a/apps/web/src/routers/organizations/organization-kiloclaw-router.ts b/apps/web/src/routers/organizations/organization-kiloclaw-router.ts index 527aaaab6e..08e1860f9a 100644 --- a/apps/web/src/routers/organizations/organization-kiloclaw-router.ts +++ b/apps/web/src/routers/organizations/organization-kiloclaw-router.ts @@ -1575,6 +1575,7 @@ export const organizationKiloclawRouter = createTRPCRouter({ path: z.string().min(1), content: z.string(), etag: z.string().min(1), + openclawValidation: z.enum(['warn-before-write', 'allow-invalid']).optional(), }) ) .mutation(async ({ ctx, input }) => { @@ -1602,6 +1603,21 @@ export const organizationKiloclawRouter = createTRPCRouter({ content = JSON.stringify(userConfig, null, 2); } + if (input.openclawValidation) { + if (input.path !== 'openclaw.json') { + throw new TRPCError({ + code: 'BAD_REQUEST', + message: 'OpenClaw validation is only available for openclaw.json', + }); + } + return await client.writeOpenclawConfigFile( + ctx.user.id, + content, + input.etag, + workerInstanceId(instance), + input.openclawValidation + ); + } return await client.writeFile( ctx.user.id, input.path, diff --git a/services/kiloclaw/controller/src/endpoint-capabilities.test.ts b/services/kiloclaw/controller/src/endpoint-capabilities.test.ts index 681a7e2577..eac431d8fc 100644 --- a/services/kiloclaw/controller/src/endpoint-capabilities.test.ts +++ b/services/kiloclaw/controller/src/endpoint-capabilities.test.ts @@ -28,6 +28,10 @@ describe('getControllerEndpointCapabilities', () => { ); }); + it('advertises validation-aware OpenClaw file writes', () => { + expect(getControllerEndpointCapabilities()).toContain('files.write-openclaw-config'); + }); + it('includes conditional Kilo Chat capabilities only when requested', () => { const defaultCapabilities = getControllerEndpointCapabilities(); const kiloChatCapabilities = getControllerEndpointCapabilities({ diff --git a/services/kiloclaw/controller/src/endpoint-capabilities.ts b/services/kiloclaw/controller/src/endpoint-capabilities.ts index fb889012ae..8bd682f9b2 100644 --- a/services/kiloclaw/controller/src/endpoint-capabilities.ts +++ b/services/kiloclaw/controller/src/endpoint-capabilities.ts @@ -17,6 +17,7 @@ export const CONTROLLER_ENDPOINT_CAPABILITIES = [ 'files.tree', 'files.read', 'files.write', + 'files.write-openclaw-config', 'files.import-openclaw-workspace', 'profile.bot-identity', 'profile.user-profile', diff --git a/services/kiloclaw/controller/src/openclaw-config-validation.test.ts b/services/kiloclaw/controller/src/openclaw-config-validation.test.ts new file mode 100644 index 0000000000..789f572b0e --- /dev/null +++ b/services/kiloclaw/controller/src/openclaw-config-validation.test.ts @@ -0,0 +1,142 @@ +import { describe, expect, it, vi } from 'vitest'; +import { + type OpenclawConfigValidationDeps, + validateOpenclawConfigCandidate, +} from './openclaw-config-validation'; + +const CONFIG_PATH = '/root/.openclaw/openclaw.json'; +const STAGE_PATH = '/root/.openclaw/.openclaw.kiloclaw-validation-candidate.json'; + +function createDeps(stdout: string, timedOut = false): OpenclawConfigValidationDeps { + return { + readCandidate: vi.fn().mockReturnValue('{"gateway":{"mode":"local"}}'), + removeFile: vi.fn(), + writeCandidate: vi.fn(), + runValidation: vi.fn().mockResolvedValue({ stdout, timedOut }), + }; +} + +describe('validateOpenclawConfigCandidate', () => { + it('validates a staged candidate and removes the stage file', async () => { + const deps = createDeps(JSON.stringify({ valid: true, path: STAGE_PATH })); + + await expect( + validateOpenclawConfigCandidate('{"gateway":{"mode":"local"}}', CONFIG_PATH, deps) + ).resolves.toEqual({ valid: true }); + + expect(deps.writeCandidate).toHaveBeenCalledWith(STAGE_PATH, '{"gateway":{"mode":"local"}}'); + expect(deps.runValidation).toHaveBeenCalledWith(STAGE_PATH); + expect(deps.removeFile).toHaveBeenNthCalledWith(1, STAGE_PATH); + expect(deps.removeFile).toHaveBeenNthCalledWith(2, `${STAGE_PATH}.bak`); + expect(deps.removeFile).toHaveBeenNthCalledWith(3, STAGE_PATH); + expect(deps.removeFile).toHaveBeenNthCalledWith(4, `${STAGE_PATH}.bak`); + }); + + it('returns bounded issues from invalid OpenClaw validation', async () => { + const deps = createDeps( + JSON.stringify({ + valid: false, + issues: [ + { path: 'gateway.mode', message: 'Unknown mode\nuse local', allowedValues: ['local'] }, + ], + }) + ); + + await expect(validateOpenclawConfigCandidate('{}', CONFIG_PATH, deps)).resolves.toEqual({ + valid: false, + reason: 'invalid', + issues: [ + { path: 'gateway.mode', message: 'Unknown mode use local', allowedValues: ['local'] }, + ], + }); + }); + + it('returns validation-unavailable when validation times out or output is unreadable', async () => { + const timeoutDeps = createDeps('', true); + await expect(validateOpenclawConfigCandidate('{}', CONFIG_PATH, timeoutDeps)).resolves.toEqual({ + valid: false, + reason: 'validation-unavailable', + issues: [{ path: '', message: 'OpenClaw configuration validation timed out.' }], + }); + + const malformedDeps = createDeps('not json'); + await expect( + validateOpenclawConfigCandidate('{}', CONFIG_PATH, malformedDeps) + ).resolves.toEqual({ + valid: false, + reason: 'validation-unavailable', + issues: [ + { path: '', message: 'OpenClaw configuration validation returned an unreadable result.' }, + ], + }); + }); + + it('redacts staging filenames from invalid diagnostics', async () => { + const deps = createDeps( + JSON.stringify({ + valid: false, + issues: [{ path: STAGE_PATH, message: `Invalid include: ${STAGE_PATH}` }], + }) + ); + + await expect(validateOpenclawConfigCandidate('{}', CONFIG_PATH, deps)).resolves.toEqual({ + valid: false, + reason: 'invalid', + issues: [{ path: 'openclaw.json', message: 'Invalid include: openclaw.json' }], + }); + }); + + it('warns if the staged bytes changed after successful validation', async () => { + const deps = createDeps(JSON.stringify({ valid: true })); + vi.mocked(deps.readCandidate).mockReturnValue('{"other":true}'); + + await expect( + validateOpenclawConfigCandidate('{"gateway":{"mode":"local"}}', CONFIG_PATH, deps) + ).resolves.toEqual({ + valid: false, + reason: 'validation-unavailable', + issues: [{ path: '', message: 'OpenClaw configuration changed during validation.' }], + }); + }); + + it('warns without staging a strict-JSON self-targeting include', async () => { + const deps = createDeps(JSON.stringify({ valid: true })); + const candidate = JSON.stringify({ agents: { $include: './openclaw.json' } }); + + await expect(validateOpenclawConfigCandidate(candidate, CONFIG_PATH, deps)).resolves.toEqual({ + valid: false, + reason: 'validation-unavailable', + issues: [ + { + path: '', + message: + 'This config includes openclaw.json itself, so it cannot be validated safely before saving.', + }, + ], + }); + expect(deps.writeCandidate).not.toHaveBeenCalled(); + expect(deps.runValidation).not.toHaveBeenCalled(); + }); + + it('warns without staging JSON5 candidates, including escaped self-targeting includes', async () => { + for (const candidate of [ + "{ agents: { $include: './openclaw.json' } }", + "{ '$incl\\u0075de': './openclaw.json' }", + ]) { + const deps = createDeps(JSON.stringify({ valid: true })); + await expect(validateOpenclawConfigCandidate(candidate, CONFIG_PATH, deps)).resolves.toEqual({ + valid: false, + reason: 'validation-unavailable', + issues: [ + { + path: '', + message: + 'This save path validates strict JSON only. Convert JSON5 syntax before saving.', + }, + ], + }); + expect(deps.writeCandidate).not.toHaveBeenCalled(); + expect(deps.runValidation).not.toHaveBeenCalled(); + } + }); +}); diff --git a/services/kiloclaw/controller/src/openclaw-config-validation.ts b/services/kiloclaw/controller/src/openclaw-config-validation.ts new file mode 100644 index 0000000000..a4328c8194 --- /dev/null +++ b/services/kiloclaw/controller/src/openclaw-config-validation.ts @@ -0,0 +1,230 @@ +import { execFile } from 'node:child_process'; +import fs from 'node:fs'; +import path from 'node:path'; +import { z } from 'zod'; + +const VALIDATION_TIMEOUT_MS = 30_000; +const VALIDATION_MAX_OUTPUT_BYTES = 1_048_576; +const VALIDATION_MAX_ISSUES = 20; +const VALIDATION_MAX_TEXT_LENGTH = 500; +export const VALIDATION_STAGE_FILENAME = '.openclaw.kiloclaw-validation-candidate.json'; + +export function isOpenclawValidationArtifactPath(relativePath: string): boolean { + return ( + relativePath === VALIDATION_STAGE_FILENAME || + relativePath.startsWith(`${VALIDATION_STAGE_FILENAME}.`) + ); +} + +const ValidationIssueSchema = z.object({ + path: z.string().optional(), + message: z.string(), + allowedValues: z.array(z.string()).optional(), +}); + +const ValidationOutputSchema = z.object({ + valid: z.boolean(), + issues: z.array(ValidationIssueSchema).optional(), + error: z.string().optional(), +}); + +export type OpenclawConfigValidationIssue = { + path: string; + message: string; + allowedValues?: string[]; +}; + +export type OpenclawConfigValidationResult = + | { valid: true } + | { + valid: false; + reason: 'invalid' | 'validation-unavailable'; + issues: OpenclawConfigValidationIssue[]; + }; + +type CommandResult = { + stdout: string; + timedOut: boolean; +}; + +export type OpenclawConfigValidationDeps = { + readCandidate: (filePath: string) => string; + removeFile: (filePath: string) => void; + writeCandidate: (filePath: string, candidate: string) => void; + runValidation: (stagePath: string) => Promise; +}; + +function isMissingFileError(error: unknown): boolean { + return typeof error === 'object' && error !== null && 'code' in error && error.code === 'ENOENT'; +} + +function removeFileIfPresent(filePath: string): void { + try { + fs.unlinkSync(filePath); + } catch (error) { + if (!isMissingFileError(error)) { + throw error; + } + } +} + +const defaultDeps: OpenclawConfigValidationDeps = { + readCandidate: filePath => fs.readFileSync(filePath, 'utf8'), + removeFile: removeFileIfPresent, + writeCandidate: (filePath, candidate) => { + fs.writeFileSync(filePath, candidate, { encoding: 'utf8', flag: 'wx', mode: 0o600 }); + }, + runValidation: stagePath => + new Promise(resolve => { + execFile( + 'openclaw', + ['config', 'validate', '--json'], + { + env: { ...process.env, OPENCLAW_CONFIG_PATH: stagePath }, + timeout: VALIDATION_TIMEOUT_MS, + maxBuffer: VALIDATION_MAX_OUTPUT_BYTES, + encoding: 'utf8', + }, + (error, stdout) => { + resolve({ stdout, timedOut: error?.killed === true }); + } + ); + }), +}; + +function clampDiagnosticText(value: string): string { + const withoutControlChars = value.replace(/[\u0000-\u001f\u007f]/g, ' ').trim(); + if (withoutControlChars.length <= VALIDATION_MAX_TEXT_LENGTH) { + return withoutControlChars; + } + return `${withoutControlChars.slice(0, VALIDATION_MAX_TEXT_LENGTH - 1)}…`; +} + +function redactStagingPath(value: string, stagePath: string): string { + return value + .replaceAll(stagePath, 'openclaw.json') + .replaceAll(path.basename(stagePath), 'openclaw.json'); +} + +function normalizeIssues( + issues: z.infer[], + stagePath: string +): OpenclawConfigValidationIssue[] { + return issues.slice(0, VALIDATION_MAX_ISSUES).map(issue => ({ + path: clampDiagnosticText(redactStagingPath(issue.path ?? '', stagePath)), + message: clampDiagnosticText(redactStagingPath(issue.message, stagePath)), + ...(issue.allowedValues + ? { + allowedValues: issue.allowedValues + .slice(0, 20) + .map(value => clampDiagnosticText(redactStagingPath(value, stagePath))), + } + : undefined), + })); +} + +function unavailableIssue(message: string): OpenclawConfigValidationResult { + return { + valid: false, + reason: 'validation-unavailable', + issues: [{ path: '', message }], + }; +} + +function referencesTargetConfig(value: unknown, configPath: string): boolean { + if (Array.isArray(value)) { + return value.some(entry => referencesTargetConfig(entry, configPath)); + } + if (typeof value !== 'object' || value === null) { + return false; + } + + for (const [key, nestedValue] of Object.entries(value)) { + if (key === '$include') { + const includes = Array.isArray(nestedValue) ? nestedValue : [nestedValue]; + if ( + includes.some( + include => + typeof include === 'string' && + path.resolve(path.dirname(configPath), include) === path.resolve(configPath) + ) + ) { + return true; + } + } + if (referencesTargetConfig(nestedValue, configPath)) { + return true; + } + } + return false; +} + +function candidateInspectionWarning(candidate: string, configPath: string): string | null { + let parsed: unknown; + try { + parsed = JSON.parse(candidate); + } catch { + return 'This save path validates strict JSON only. Convert JSON5 syntax before saving.'; + } + if (referencesTargetConfig(parsed, configPath)) { + return 'This config includes openclaw.json itself, so it cannot be validated safely before saving.'; + } + return null; +} + +export async function validateOpenclawConfigCandidate( + candidate: string, + configPath: string, + deps: OpenclawConfigValidationDeps = defaultDeps +): Promise { + const inspectionWarning = candidateInspectionWarning(candidate, configPath); + if (inspectionWarning) { + return unavailableIssue(inspectionWarning); + } + + const stagePath = path.join(path.dirname(configPath), VALIDATION_STAGE_FILENAME); + const stageBackupPath = `${stagePath}.bak`; + try { + deps.removeFile(stagePath); + deps.removeFile(stageBackupPath); + deps.writeCandidate(stagePath, candidate); + const commandResult = await deps.runValidation(stagePath); + if (commandResult.timedOut) { + return unavailableIssue('OpenClaw configuration validation timed out.'); + } + + let rawResult: unknown; + try { + rawResult = JSON.parse(commandResult.stdout); + } catch { + return unavailableIssue('OpenClaw configuration validation returned an unreadable result.'); + } + const result = ValidationOutputSchema.safeParse(rawResult); + if (!result.success) { + return unavailableIssue('OpenClaw configuration validation returned an unreadable result.'); + } + if (result.data.valid) { + if (deps.readCandidate(stagePath) !== candidate) { + return unavailableIssue('OpenClaw configuration changed during validation.'); + } + return { valid: true }; + } + if (result.data.issues && result.data.issues.length > 0) { + return { + valid: false, + reason: 'invalid', + issues: normalizeIssues(result.data.issues, stagePath), + }; + } + return unavailableIssue('OpenClaw could not validate this configuration.'); + } catch { + return unavailableIssue('OpenClaw configuration validation could not be started.'); + } finally { + try { + deps.removeFile(stagePath); + deps.removeFile(stageBackupPath); + } catch { + // Best-effort cleanup: never replace a validation outcome with cleanup failure. + } + } +} diff --git a/services/kiloclaw/controller/src/routes/files.test.ts b/services/kiloclaw/controller/src/routes/files.test.ts index 5edace8214..d9b5b308ba 100644 --- a/services/kiloclaw/controller/src/routes/files.test.ts +++ b/services/kiloclaw/controller/src/routes/files.test.ts @@ -24,9 +24,16 @@ vi.mock('../backup-file', () => ({ backupFile: vi.fn(), })); +vi.mock('../openclaw-config-validation', () => ({ + isOpenclawValidationArtifactPath: (relativePath: string) => + relativePath.startsWith('.openclaw.kiloclaw-validation-candidate.json'), + validateOpenclawConfigCandidate: vi.fn(), +})); + import fs from 'node:fs'; import { atomicWrite } from '../atomic-write'; import { backupFile } from '../backup-file'; +import { validateOpenclawConfigCandidate } from '../openclaw-config-validation'; const TOKEN = 'test-token'; const ROOT = '/root/.openclaw'; @@ -156,6 +163,7 @@ describe('file routes', () => { mockDirent('credentials', true), mockDirent('SOUL.md.bak.2026-03-01', false), mockDirent('debug.log', false), + mockDirent('.openclaw.kiloclaw-validation-candidate.json', false), ] as any; } if (dir === `${ROOT}/workspace`) { @@ -180,6 +188,7 @@ describe('file routes', () => { expect(names).toContain('SOUL.md'); expect(names).toContain('credentials'); expect(names).toContain('token.txt'); + expect(names).not.toContain('.openclaw.kiloclaw-validation-candidate.json'); }); it('skips symlinks', async () => { @@ -338,6 +347,130 @@ describe('file routes', () => { expect(body.etag).toBeDefined(); }); + it('writes a validation-aware valid openclaw config with restricted mode', async () => { + vi.mocked(fs.existsSync).mockReturnValue(true); + vi.mocked(fs.lstatSync).mockReturnValue({ + isSymbolicLink: () => false, + isFile: () => true, + } as any); + vi.mocked(fs.readFileSync).mockReturnValue('old config'); + vi.mocked(validateOpenclawConfigCandidate).mockResolvedValue({ valid: true }); + + const res = await app.request('/_kilo/files/write-openclaw-config', { + method: 'POST', + headers: authHeaders(), + body: JSON.stringify({ + content: '{"gateway":{"mode":"local"}}', + mode: 'warn-before-write', + }), + }); + + expect(res.status).toBe(200); + expect(validateOpenclawConfigCandidate).toHaveBeenCalledWith( + '{"gateway":{"mode":"local"}}', + `${ROOT}/openclaw.json` + ); + expect(atomicWrite).toHaveBeenCalledWith( + `${ROOT}/openclaw.json`, + '{"gateway":{"mode":"local"}}', + undefined, + { mode: 0o600 } + ); + }); + + it('returns a warning without writing an invalid openclaw config', async () => { + vi.mocked(fs.existsSync).mockReturnValue(true); + vi.mocked(fs.lstatSync).mockReturnValue({ + isSymbolicLink: () => false, + isFile: () => true, + } as any); + vi.mocked(validateOpenclawConfigCandidate).mockResolvedValue({ + valid: false, + reason: 'invalid', + issues: [{ path: 'gateway.mode', message: 'Expected local' }], + }); + + const res = await app.request('/_kilo/files/write-openclaw-config', { + method: 'POST', + headers: authHeaders(), + body: JSON.stringify({ + content: '{"gateway":{"mode":"remote"}}', + mode: 'warn-before-write', + }), + }); + + expect(res.status).toBe(200); + await expect(res.json()).resolves.toEqual({ + outcome: 'openclaw-validation-warning', + valid: false, + reason: 'invalid', + issues: [{ path: 'gateway.mode', message: 'Expected local' }], + }); + expect(backupFile).not.toHaveBeenCalled(); + expect(atomicWrite).not.toHaveBeenCalled(); + }); + + it('allows an explicit invalid openclaw config override', async () => { + vi.mocked(fs.existsSync).mockReturnValue(true); + vi.mocked(fs.lstatSync).mockReturnValue({ + isSymbolicLink: () => false, + isFile: () => true, + } as any); + + const res = await app.request('/_kilo/files/write-openclaw-config', { + method: 'POST', + headers: authHeaders(), + body: JSON.stringify({ + content: '{"gateway":{"mode":"remote"}}', + mode: 'allow-invalid', + }), + }); + + expect(res.status).toBe(200); + expect(validateOpenclawConfigCandidate).not.toHaveBeenCalled(); + expect(atomicWrite).toHaveBeenCalledWith( + `${ROOT}/openclaw.json`, + '{"gateway":{"mode":"remote"}}', + undefined, + { mode: 0o600 } + ); + }); + + it('hides internal validation artifacts and normalized aliases from generic writes', async () => { + for (const candidatePath of [ + '.openclaw.kiloclaw-validation-candidate.json', + './.openclaw.kiloclaw-validation-candidate.json', + 'workspace/../.openclaw.kiloclaw-validation-candidate.json', + ]) { + const res = await app.request('/_kilo/files/write', { + method: 'POST', + headers: authHeaders(), + body: JSON.stringify({ path: candidatePath, content: 'new content' }), + }); + expect(res.status).toBe(404); + } + expect(validateOpenclawConfigCandidate).not.toHaveBeenCalled(); + }); + + it('hides validation artifacts reached through a symlinked directory alias', async () => { + vi.mocked(fs.existsSync).mockReturnValue(true); + vi.mocked(fs.realpathSync).mockImplementationOnce( + () => `${ROOT}/.openclaw.kiloclaw-validation-candidate.json` + ); + + const res = await app.request('/_kilo/files/write', { + method: 'POST', + headers: authHeaders(), + body: JSON.stringify({ + path: 'alias/.openclaw.kiloclaw-validation-candidate.json', + content: 'new content', + }), + }); + + expect(res.status).toBe(404); + expect(atomicWrite).not.toHaveBeenCalled(); + }); + it('path traversal still rejected', async () => { const res = await app.request('/_kilo/files/write', { method: 'POST', diff --git a/services/kiloclaw/controller/src/routes/files.ts b/services/kiloclaw/controller/src/routes/files.ts index c93c784e09..921e69fab4 100644 --- a/services/kiloclaw/controller/src/routes/files.ts +++ b/services/kiloclaw/controller/src/routes/files.ts @@ -16,6 +16,11 @@ import { timingSafeTokenEqual } from '../auth'; import { resolveSafePath, verifyCanonicalized, SafePathError } from '../safe-path'; import { atomicWrite } from '../atomic-write'; import { backupFile } from '../backup-file'; +import { serializeAgentConfigMutation } from '../openclaw-agent-config'; +import { + isOpenclawValidationArtifactPath, + validateOpenclawConfigCandidate, +} from '../openclaw-config-validation'; import { ensureWeatherSkillInstalled, formatBotIdentityMarkdown, @@ -317,6 +322,7 @@ function buildTree(dir: string, rootDir: string): FileNode[] { if (entry.isSymbolicLink()) continue; const relativePath = path.relative(rootDir, path.join(dir, entry.name)); + if (isOpenclawValidationArtifactPath(relativePath)) continue; if (entry.isDirectory()) { nodes.push({ @@ -358,19 +364,28 @@ function resolveAndValidateFile( throw e; } + if (isOpenclawValidationArtifactPath(path.relative(rootDir, resolved))) { + return { code: 'file_not_found', error: 'File does not exist', status: 404 }; + } + if (!fs.existsSync(resolved)) { return { code: 'file_not_found', error: 'File does not exist', status: 404 }; } - // Canonicalize to catch symlinked ancestors escaping the root + // Canonicalize to catch symlinked ancestors escaping the root or aliasing internal artifacts. + let canonicalPath: string; try { - verifyCanonicalized(fs.realpathSync(resolved), rootDir); + canonicalPath = fs.realpathSync(resolved); + verifyCanonicalized(canonicalPath, rootDir); } catch (e) { if (e instanceof SafePathError) { return { error: e.message, status: 400 }; } throw e; } + if (isOpenclawValidationArtifactPath(path.relative(rootDir, canonicalPath))) { + return { code: 'file_not_found', error: 'File does not exist', status: 404 }; + } const stat = fs.lstatSync(resolved); if (stat.isSymbolicLink()) { @@ -697,7 +712,6 @@ export function registerFileRoutes(app: Hono, expectedToken: string, rootDir: st return c.json({ error: 'Missing or invalid path/content' }, 400); } const body = parsed.data; - const result = resolveAndValidateFile(body.path, rootDir); if (typeof result !== 'string') { return c.json( @@ -708,8 +722,7 @@ export function registerFileRoutes(app: Hono, expectedToken: string, rootDir: st if (body.etag) { const currentContent = fs.readFileSync(result, 'utf-8'); - const currentEtag = computeEtag(currentContent); - if (body.etag !== currentEtag) { + if (body.etag !== computeEtag(currentContent)) { return c.json({ code: 'file_etag_conflict', error: 'File was modified externally' }, 409); } } @@ -725,8 +738,73 @@ export function registerFileRoutes(app: Hono, expectedToken: string, rootDir: st console.error('[files] atomicWrite failed:', err); return c.json({ error: 'Failed to write file' }, 500); } + return c.json({ etag: computeEtag(body.content) }); + }); + + const WriteOpenclawConfigBodySchema = z.object({ + content: z.string(), + etag: z.string().optional(), + mode: z.enum(['warn-before-write', 'allow-invalid']), + }); + + app.post('/_kilo/files/write-openclaw-config', async c => { + let rawBody: unknown; + try { + rawBody = await c.req.json(); + } catch { + return c.json({ error: 'Invalid JSON body' }, 400); + } + const parsed = WriteOpenclawConfigBodySchema.safeParse(rawBody); + if (!parsed.success) { + return c.json({ error: 'Missing or invalid content/mode' }, 400); + } + const body = parsed.data; + const result = resolveAndValidateFile('openclaw.json', rootDir); + if (typeof result !== 'string') { + return c.json( + { error: result.error, ...(result.code && { code: result.code }) }, + result.status + ); + } + + const hasEtagConflict = () => { + if (!body.etag) return false; + const currentContent = fs.readFileSync(result, 'utf-8'); + return body.etag !== computeEtag(currentContent); + }; + + return serializeAgentConfigMutation( + async () => { + if (hasEtagConflict()) { + return c.json({ code: 'file_etag_conflict', error: 'File was modified externally' }, 409); + } + if (body.mode === 'warn-before-write') { + const validation = await validateOpenclawConfigCandidate(body.content, result); + if (!validation.valid) { + return c.json({ outcome: 'openclaw-validation-warning', ...validation }); + } + if (hasEtagConflict()) { + return c.json( + { code: 'file_etag_conflict', error: 'File was modified externally' }, + 409 + ); + } + } - const newEtag = computeEtag(body.content); - return c.json({ etag: newEtag }); + try { + backupFile(result, rootDir); + } catch (err) { + console.warn('[files] Failed to create backup, proceeding with write:', err); + } + try { + atomicWrite(result, body.content, undefined, { mode: 0o600 }); + } catch (err) { + console.error('[files] atomicWrite failed:', err); + return c.json({ error: 'Failed to write file' }, 500); + } + return c.json({ etag: computeEtag(body.content) }); + }, + { configPath: result } + ); }); } diff --git a/services/kiloclaw/scripts/controller-entrypoint-smoke-test.sh b/services/kiloclaw/scripts/controller-entrypoint-smoke-test.sh index 953f1b2bd8..b8a572af08 100644 --- a/services/kiloclaw/scripts/controller-entrypoint-smoke-test.sh +++ b/services/kiloclaw/scripts/controller-entrypoint-smoke-test.sh @@ -169,6 +169,20 @@ else check "packaged no-trash baseline retains deleted workspace" "1" "0" fi +echo +echo "--- validation-aware openclaw.json write ---" + +CONFIG_SHA_BEFORE=$(python3 -c "import hashlib; print(hashlib.sha256(open('$ROOTDIR/.openclaw/openclaw.json','rb').read()).hexdigest())") +INVALID_WRITE_RESP=$(curl -sS -X POST \ + -H "Authorization: Bearer $TOKEN" \ + -H "Content-Type: application/json" \ + -d '{"content":"{\"unexpected_root_key\":true}","mode":"warn-before-write"}' \ + "http://127.0.0.1:${PORT}/_kilo/files/write-openclaw-config") +INVALID_WRITE_OUTCOME=$(echo "$INVALID_WRITE_RESP" | python3 -c "import sys,json; print(json.load(sys.stdin).get('outcome',''))" 2>/dev/null || echo "") +CONFIG_SHA_AFTER=$(python3 -c "import hashlib; print(hashlib.sha256(open('$ROOTDIR/.openclaw/openclaw.json','rb').read()).hexdigest())") +check "invalid openclaw write returns validation warning" "openclaw-validation-warning" "$INVALID_WRITE_OUTCOME" +check "invalid openclaw warning leaves config unchanged" "$CONFIG_SHA_BEFORE" "$CONFIG_SHA_AFTER" + assert_kilo_chat_smoke "$CID" "$PORT" "$TOKEN" echo diff --git a/services/kiloclaw/src/durable-objects/gateway-controller-types.test.ts b/services/kiloclaw/src/durable-objects/gateway-controller-types.test.ts index b08fc2ba70..f9baab7e4f 100644 --- a/services/kiloclaw/src/durable-objects/gateway-controller-types.test.ts +++ b/services/kiloclaw/src/durable-objects/gateway-controller-types.test.ts @@ -1,5 +1,22 @@ import { describe, expect, it } from 'vitest'; -import { ControllerVersionResponseSchema } from './gateway-controller-types'; +import { + ControllerVersionResponseSchema, + FileWriteResponseSchema, +} from './gateway-controller-types'; + +describe('FileWriteResponseSchema', () => { + it('accepts written and validation-warning results', () => { + expect(FileWriteResponseSchema.parse({ etag: 'etag-1' })).toEqual({ etag: 'etag-1' }); + expect( + FileWriteResponseSchema.parse({ + outcome: 'openclaw-validation-warning', + valid: false, + reason: 'invalid', + issues: [{ path: 'gateway.mode', message: 'Expected local', allowedValues: ['local'] }], + }) + ).toMatchObject({ outcome: 'openclaw-validation-warning', reason: 'invalid' }); + }); +}); describe('ControllerVersionResponseSchema', () => { it('accepts legacy version responses without capability hints', () => { diff --git a/services/kiloclaw/src/durable-objects/gateway-controller-types.ts b/services/kiloclaw/src/durable-objects/gateway-controller-types.ts index b3595f9f96..768f70c8e9 100644 --- a/services/kiloclaw/src/durable-objects/gateway-controller-types.ts +++ b/services/kiloclaw/src/durable-objects/gateway-controller-types.ts @@ -234,6 +234,26 @@ export const OpenclawConfigResponseSchema = z.object({ etag: z.string(), }); +export const OpenclawFileWriteValidationSchema = z.enum(['warn-before-write', 'allow-invalid']); +export type OpenclawFileWriteValidation = z.infer; + +const OpenclawValidationIssueSchema = z.object({ + path: z.string(), + message: z.string(), + allowedValues: z.array(z.string()).optional(), +}); + +export const FileWriteResponseSchema = z.union([ + z.object({ etag: z.string() }), + z.object({ + outcome: z.literal('openclaw-validation-warning'), + valid: z.literal(false), + reason: z.enum(['invalid', 'validation-unavailable']), + issues: z.array(OpenclawValidationIssueSchema), + }), +]); +export type FileWriteResponse = z.infer; + // ────────────────────────────────────────────────────────────────────── // Controller pairing responses // diff --git a/services/kiloclaw/src/durable-objects/kiloclaw-instance/gateway.test.ts b/services/kiloclaw/src/durable-objects/kiloclaw-instance/gateway.test.ts index 92027acb88..2a8e679f00 100644 --- a/services/kiloclaw/src/durable-objects/kiloclaw-instance/gateway.test.ts +++ b/services/kiloclaw/src/durable-objects/kiloclaw-instance/gateway.test.ts @@ -6,6 +6,7 @@ import { getMorningBriefingStatus, runMorningBriefing, waitForHealthy, + writeOpenclawConfigFile, } from './gateway'; import { GatewayControllerError } from '../gateway-controller-types'; @@ -220,6 +221,47 @@ describe('gateway controller routing', () => { expect(timeoutSpy).toHaveBeenCalledWith(120_000); }); + it('forwards validation-aware file writes and parses warnings', async () => { + const state = createMutableState(); + state.provider = 'fly'; + state.status = 'running'; + state.sandboxId = 'sandbox-1'; + state.flyAppName = 'test-app'; + state.flyMachineId = 'machine-1'; + + const fetchMock: FetchMock = vi.fn().mockResolvedValue( + new Response( + JSON.stringify({ + outcome: 'openclaw-validation-warning', + valid: false, + reason: 'invalid', + issues: [{ path: 'gateway.mode', message: 'Expected local' }], + }), + { status: 200, headers: { 'content-type': 'application/json' } } + ) + ); + vi.stubGlobal('fetch', fetchMock); + + const result = await writeOpenclawConfigFile( + state, + { GATEWAY_TOKEN_SECRET: 'gateway-secret', FLY_APP_NAME: 'fallback-app' } as never, + '{"gateway":{"mode":"remote"}}', + 'etag-1', + 'warn-before-write' + ); + + expect(result).toMatchObject({ outcome: 'openclaw-validation-warning', reason: 'invalid' }); + const { init } = getFetchCall(fetchMock); + if (typeof init?.body !== 'string') { + throw new Error('Expected JSON string request body'); + } + expect(JSON.parse(init.body)).toEqual({ + content: '{"gateway":{"mode":"remote"}}', + etag: 'etag-1', + mode: 'warn-before-write', + }); + }); + it('fails controller RPCs before fetching when instance state is not running', async () => { const state = createMutableState(); state.provider = 'fly'; diff --git a/services/kiloclaw/src/durable-objects/kiloclaw-instance/gateway.ts b/services/kiloclaw/src/durable-objects/kiloclaw-instance/gateway.ts index 4d8e208d0e..9610151032 100644 --- a/services/kiloclaw/src/durable-objects/kiloclaw-instance/gateway.ts +++ b/services/kiloclaw/src/durable-objects/kiloclaw-instance/gateway.ts @@ -13,6 +13,9 @@ import { EnvPatchResponseSchema, ToolsMdSectionSyncResponseSchema, OpenclawConfigResponseSchema, + FileWriteResponseSchema as ValidationAwareFileWriteResponseSchema, + type FileWriteResponse, + type OpenclawFileWriteValidation, MorningBriefingStatusResponseSchema, MorningBriefingActionResponseSchema, MorningBriefingInterestsResponseSchema, @@ -454,9 +457,7 @@ export async function readFile( } } -const FileWriteResponseSchema = z.object({ - etag: z.string(), -}); +const LegacyFileWriteResponseSchema = z.object({ etag: z.string() }); export async function writeFile( state: InstanceMutableState, @@ -471,7 +472,7 @@ export async function writeFile( env, '/_kilo/files/write', 'POST', - FileWriteResponseSchema, + LegacyFileWriteResponseSchema, { path: filePath, content, etag } ); } catch (error) { @@ -480,6 +481,28 @@ export async function writeFile( } } +export async function writeOpenclawConfigFile( + state: InstanceMutableState, + env: KiloClawEnv, + content: string, + etag: string | undefined, + mode: OpenclawFileWriteValidation +): Promise { + try { + return await callGatewayController( + state, + env, + '/_kilo/files/write-openclaw-config', + 'POST', + ValidationAwareFileWriteResponseSchema, + { content, etag, mode } + ); + } catch (error) { + if (isErrorUnknownRoute(error)) return null; + throw error; + } +} + export async function importOpenclawWorkspace( state: InstanceMutableState, env: KiloClawEnv, diff --git a/services/kiloclaw/src/durable-objects/kiloclaw-instance/index.ts b/services/kiloclaw/src/durable-objects/kiloclaw-instance/index.ts index 36d2e56f51..17b150cd42 100644 --- a/services/kiloclaw/src/durable-objects/kiloclaw-instance/index.ts +++ b/services/kiloclaw/src/durable-objects/kiloclaw-instance/index.ts @@ -10,6 +10,7 @@ import { DurableObject } from 'cloudflare:workers'; import type { KiloClawEnv } from '../../types'; +import type { OpenclawFileWriteValidation } from '../gateway-controller-types'; import type { InstanceConfig, PersistedState, @@ -3765,6 +3766,15 @@ export class KiloClawInstance extends DurableObject { return gateway.writeFile(this.s, this.env, filePath, content, etag); } + async writeOpenclawConfigFile( + content: string, + etag: string | undefined, + mode: OpenclawFileWriteValidation + ) { + await this.loadState(); + return gateway.writeOpenclawConfigFile(this.s, this.env, content, etag, mode); + } + async importOpenclawWorkspace(files: Array<{ path: string; content: string }>) { await this.loadState(); return gateway.importOpenclawWorkspace(this.s, this.env, files); diff --git a/services/kiloclaw/src/routes/platform-sanitize-error.test.ts b/services/kiloclaw/src/routes/platform-sanitize-error.test.ts index a5b80afc67..d5481071d9 100644 --- a/services/kiloclaw/src/routes/platform-sanitize-error.test.ts +++ b/services/kiloclaw/src/routes/platform-sanitize-error.test.ts @@ -757,6 +757,87 @@ describe('sanitizeError: explicit provider support errors', () => { }); }); +describe('validation-aware config write platform route', () => { + function envWithWriteOpenclawConfigFile( + writeOpenclawConfigFile: (...args: unknown[]) => Promise + ) { + return { + KILOCLAW_INSTANCE: { + idFromName: (id: string) => id, + get: () => ({ writeOpenclawConfigFile }), + }, + KILOCLAW_AE: { writeDataPoint: vi.fn() }, + KV_CLAW_CACHE: { + get: vi.fn().mockResolvedValue(null), + put: vi.fn().mockResolvedValue(undefined), + delete: vi.fn().mockResolvedValue(undefined), + list: vi.fn().mockResolvedValue({ keys: [], list_complete: true }), + getWithMetadata: vi.fn().mockResolvedValue({ value: null, metadata: null }), + }, + } as never; + } + + it('forwards opt-in validation mode and warning responses', async () => { + const writeOpenclawConfigFile = vi.fn().mockResolvedValue({ + outcome: 'openclaw-validation-warning', + valid: false, + reason: 'invalid', + issues: [{ path: 'gateway.mode', message: 'Expected local' }], + }); + const env = envWithWriteOpenclawConfigFile(writeOpenclawConfigFile); + + const resp = await platform.request( + '/files/write-openclaw-config?instanceId=11111111-1111-4111-8111-111111111111', + { + method: 'POST', + headers: { 'content-type': 'application/json' }, + body: JSON.stringify({ + userId: 'user-1', + content: '{"gateway":{"mode":"remote"}}', + etag: 'etag-1', + mode: 'warn-before-write', + }), + }, + env + ); + + expect(resp.status).toBe(200); + await expect(jsonBody(resp)).resolves.toMatchObject({ + outcome: 'openclaw-validation-warning', + reason: 'invalid', + }); + expect(writeOpenclawConfigFile).toHaveBeenCalledWith( + '{"gateway":{"mode":"remote"}}', + 'etag-1', + 'warn-before-write' + ); + }); + + it('fails closed when the running controller lacks the dedicated route', async () => { + const env = envWithWriteOpenclawConfigFile(async () => null); + const resp = await platform.request( + '/files/write-openclaw-config', + { + method: 'POST', + headers: { 'content-type': 'application/json' }, + body: JSON.stringify({ + userId: 'user-1', + content: '{"gateway":{"mode":"remote"}}', + etag: 'etag-1', + mode: 'warn-before-write', + }), + }, + env + ); + + expect(resp.status).toBe(404); + await expect(jsonBody(resp)).resolves.toEqual({ + error: 'OpenClaw validation-aware writing not available (controller too old)', + code: 'controller_route_unavailable', + }); + }); +}); + describe('openclaw import platform route', () => { function envWithImportOpenclawWorkspace( importOpenclawWorkspace: (files: Array<{ path: string; content: string }>) => Promise, diff --git a/services/kiloclaw/src/routes/platform.ts b/services/kiloclaw/src/routes/platform.ts index ab2264c4dd..75fdf565ba 100644 --- a/services/kiloclaw/src/routes/platform.ts +++ b/services/kiloclaw/src/routes/platform.ts @@ -9,6 +9,7 @@ import { Hono } from 'hono'; import type { Context } from 'hono'; import * as fly from '../fly/client'; import type { InstanceStatus } from '../durable-objects/kiloclaw-instance/types'; +import type { FileWriteResponse } from '../durable-objects/gateway-controller-types'; import type { AppEnv } from '../types'; import { ProvisionRequestSchema, @@ -2717,6 +2718,13 @@ const WriteFileSchema = z.object({ etag: z.string().optional(), }); +const WriteOpenclawConfigFileSchema = z.object({ + userId: z.string().min(1), + content: z.string(), + etag: z.string().optional(), + mode: z.enum(['warn-before-write', 'allow-invalid']), +}); + const OpenclawWorkspaceImportSchema = z.object({ userId: z.string().min(1), files: z @@ -2761,6 +2769,40 @@ platform.post('/files/write', async c => { } }); +// POST /api/platform/files/write-openclaw-config +platform.post('/files/write-openclaw-config', async c => { + const result = await parseBody(c, WriteOpenclawConfigFileSchema); + if ('error' in result) return result.error; + + const iidResult = parseInstanceIdQuery(c); + if ('error' in iidResult) return iidResult.error; + + const { userId, content, etag, mode } = result.data; + try { + const response = await withResolvedDORetry( + c.env, + userId, + iidResult.instanceId, + stub => stub.writeOpenclawConfigFile(content, etag, mode), + 'writeOpenclawConfigFile' + ); + if (!response) { + return jsonError( + 'OpenClaw validation-aware writing not available (controller too old)', + 404, + 'controller_route_unavailable' + ); + } + return c.json(response, 200); + } catch (err) { + const { message, status, code } = sanitizeOpenclawConfigError( + err, + 'files/write-openclaw-config' + ); + return jsonError(message, status, code); + } +}); + // POST /api/platform/files/import-openclaw-workspace platform.post('/files/import-openclaw-workspace', async c => { const result = await parseBody(c, OpenclawWorkspaceImportSchema); From 5b39c0af01228ca53c1fd6b1e74755d12623f783 Mon Sep 17 00:00:00 2001 From: syn Date: Wed, 27 May 2026 15:22:04 -0500 Subject: [PATCH 2/5] fix(kiloclaw): harden config validation save flow --- .specs/kiloclaw-controller.md | 2 +- apps/web/src/app/(app)/claw/components/FileEditorPane.tsx | 8 +++++++- 2 files changed, 8 insertions(+), 2 deletions(-) diff --git a/.specs/kiloclaw-controller.md b/.specs/kiloclaw-controller.md index 7d74151173..f0c3fc2c0f 100644 --- a/.specs/kiloclaw-controller.md +++ b/.specs/kiloclaw-controller.md @@ -738,7 +738,7 @@ running. When forwarding, it MUST authenticate to gog with 2. For a normal validation-aware save, the controller MUST evaluate the submitted candidate using the installed OpenClaw config-validation behavior before committing it. 3. When the candidate is invalid or validation cannot complete, the controller MUST leave `openclaw.json` unchanged and MUST return a bounded structured warning result suitable for an authenticated client to display. 4. The controller MAY accept an explicit invalid-write override after a warning. An override MUST remain subject to normal safe-path and ETag concurrency checks and MUST NOT be inferred from an ordinary save request. -5. Validation-warning and override writes MUST NOT expose config contents, secrets, environment values, staging paths, or unrestricted subprocess output in responses or logs. +5. Validation-warning responses MAY return bounded diagnostics derived while validating the authenticated instance's configuration, including substituted values; those diagnostics MUST be returned only to authenticated config-management clients and MUST NOT be logged. Responses and logs MUST NOT expose staging paths or unrestricted subprocess output. 6. Validation-aware writes MUST remain usable when the gateway process is unavailable after controller routes are registered. 7. Controllers implementing this behavior MUST advertise `files.write-openclaw-config`; clients MUST NOT infer this behavior solely from controller CalVer. 8. This validation reports OpenClaw configuration validity. It MUST NOT be represented as proof that runtime SecretRefs or optional environment substitutions resolve successfully. diff --git a/apps/web/src/app/(app)/claw/components/FileEditorPane.tsx b/apps/web/src/app/(app)/claw/components/FileEditorPane.tsx index a6d38ffeb9..7e641cb333 100644 --- a/apps/web/src/app/(app)/claw/components/FileEditorPane.tsx +++ b/apps/web/src/app/(app)/claw/components/FileEditorPane.tsx @@ -176,6 +176,7 @@ export function FileEditorPane({ }, onError: err => { if (err.data?.code === 'CONFLICT' && err.data?.upstreamCode === 'file_etag_conflict') { + setPendingValidationWarning(null); refetch(); setShowDiff(true); toast.error( @@ -257,7 +258,7 @@ export function FileEditorPane({ value={currentValue} onChange={handleEditorChange} theme="vs-dark" - options={EDITOR_OPTIONS} + options={{ ...EDITOR_OPTIONS, readOnly: isSaving }} keepCurrentModel /> )} @@ -331,6 +332,11 @@ export function FileEditorPane({
  • {issue.path &&
    {issue.path}
    }
    {issue.message}
    + {issue.allowedValues && issue.allowedValues.length > 0 && ( +
    + Allowed values: {issue.allowedValues.join(', ')} +
    + )}
    • ))} From f809cec43077d4360afb6cd3ae7265f0d4cb87d7 Mon Sep 17 00:00:00 2001 From: syn Date: Wed, 27 May 2026 15:39:47 -0500 Subject: [PATCH 3/5] fix(kiloclaw): handle config validation failures --- .../src/openclaw-config-validation.test.ts | 21 ++++++++++++++ .../src/openclaw-config-validation.ts | 21 +++++++++++--- .../controller/src/routes/files.test.ts | 29 +++++++++++++++++++ .../kiloclaw/controller/src/routes/files.ts | 8 +++-- 4 files changed, 73 insertions(+), 6 deletions(-) diff --git a/services/kiloclaw/controller/src/openclaw-config-validation.test.ts b/services/kiloclaw/controller/src/openclaw-config-validation.test.ts index 789f572b0e..9dba79fabf 100644 --- a/services/kiloclaw/controller/src/openclaw-config-validation.test.ts +++ b/services/kiloclaw/controller/src/openclaw-config-validation.test.ts @@ -71,6 +71,27 @@ describe('validateOpenclawConfigCandidate', () => { }); }); + it('logs safe failure metadata when staging fails unexpectedly', async () => { + const deps = createDeps(JSON.stringify({ valid: true })); + const log = vi.spyOn(console, 'error').mockImplementation(() => undefined); + vi.mocked(deps.writeCandidate).mockImplementation(() => { + throw Object.assign(new Error('disk full at a sensitive path'), { code: 'ENOSPC' }); + }); + + await expect(validateOpenclawConfigCandidate('{}', CONFIG_PATH, deps)).resolves.toEqual({ + valid: false, + reason: 'validation-unavailable', + issues: [{ path: '', message: 'OpenClaw configuration validation could not be started.' }], + }); + expect(log).toHaveBeenCalledWith( + '[openclaw-config-validation] Validation failed unexpectedly:', + 'ENOSPC' + ); + expect(log.mock.calls.flat().join(' ')).not.toContain('sensitive path'); + + log.mockRestore(); + }); + it('redacts staging filenames from invalid diagnostics', async () => { const deps = createDeps( JSON.stringify({ diff --git a/services/kiloclaw/controller/src/openclaw-config-validation.ts b/services/kiloclaw/controller/src/openclaw-config-validation.ts index a4328c8194..c8b7dd1b6c 100644 --- a/services/kiloclaw/controller/src/openclaw-config-validation.ts +++ b/services/kiloclaw/controller/src/openclaw-config-validation.ts @@ -54,8 +54,20 @@ export type OpenclawConfigValidationDeps = { runValidation: (stagePath: string) => Promise; }; +function errorCode(error: unknown): string { + if ( + typeof error === 'object' && + error !== null && + 'code' in error && + typeof error.code === 'string' + ) { + return error.code; + } + return 'unknown'; +} + function isMissingFileError(error: unknown): boolean { - return typeof error === 'object' && error !== null && 'code' in error && error.code === 'ENOENT'; + return errorCode(error) === 'ENOENT'; } function removeFileIfPresent(filePath: string): void { @@ -217,14 +229,15 @@ export async function validateOpenclawConfigCandidate( }; } return unavailableIssue('OpenClaw could not validate this configuration.'); - } catch { + } catch (error) { + console.error('[openclaw-config-validation] Validation failed unexpectedly:', errorCode(error)); return unavailableIssue('OpenClaw configuration validation could not be started.'); } finally { try { deps.removeFile(stagePath); deps.removeFile(stageBackupPath); - } catch { - // Best-effort cleanup: never replace a validation outcome with cleanup failure. + } catch (error) { + console.warn('[openclaw-config-validation] Staging cleanup failed:', errorCode(error)); } } } diff --git a/services/kiloclaw/controller/src/routes/files.test.ts b/services/kiloclaw/controller/src/routes/files.test.ts index d9b5b308ba..febf92f01c 100644 --- a/services/kiloclaw/controller/src/routes/files.test.ts +++ b/services/kiloclaw/controller/src/routes/files.test.ts @@ -378,6 +378,35 @@ describe('file routes', () => { ); }); + it('returns a conflict if openclaw.json disappears during ETag validation', async () => { + vi.mocked(fs.existsSync).mockReturnValue(true); + vi.mocked(fs.lstatSync).mockReturnValue({ + isSymbolicLink: () => false, + isFile: () => true, + } as any); + vi.mocked(fs.readFileSync).mockImplementation(() => { + throw Object.assign(new Error('file removed'), { code: 'ENOENT' }); + }); + + const res = await app.request('/_kilo/files/write-openclaw-config', { + method: 'POST', + headers: authHeaders(), + body: JSON.stringify({ + content: '{"gateway":{"mode":"local"}}', + etag: 'prior-etag', + mode: 'warn-before-write', + }), + }); + + expect(res.status).toBe(409); + await expect(res.json()).resolves.toEqual({ + code: 'file_etag_conflict', + error: 'File was modified externally', + }); + expect(validateOpenclawConfigCandidate).not.toHaveBeenCalled(); + expect(atomicWrite).not.toHaveBeenCalled(); + }); + it('returns a warning without writing an invalid openclaw config', async () => { vi.mocked(fs.existsSync).mockReturnValue(true); vi.mocked(fs.lstatSync).mockReturnValue({ diff --git a/services/kiloclaw/controller/src/routes/files.ts b/services/kiloclaw/controller/src/routes/files.ts index 921e69fab4..627be6ff67 100644 --- a/services/kiloclaw/controller/src/routes/files.ts +++ b/services/kiloclaw/controller/src/routes/files.ts @@ -769,8 +769,12 @@ export function registerFileRoutes(app: Hono, expectedToken: string, rootDir: st const hasEtagConflict = () => { if (!body.etag) return false; - const currentContent = fs.readFileSync(result, 'utf-8'); - return body.etag !== computeEtag(currentContent); + try { + const currentContent = fs.readFileSync(result, 'utf-8'); + return body.etag !== computeEtag(currentContent); + } catch { + return true; + } }; return serializeAgentConfigMutation( From 6dbd070c2ee06d640e11d7b5570d95d301a6caee Mon Sep 17 00:00:00 2001 From: syn Date: Wed, 27 May 2026 16:48:10 -0500 Subject: [PATCH 4/5] docs(kiloclaw): clarify config validation changelog entry --- apps/web/src/app/(app)/claw/components/changelog-data.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/apps/web/src/app/(app)/claw/components/changelog-data.ts b/apps/web/src/app/(app)/claw/components/changelog-data.ts index 449511ace4..3069a05be3 100644 --- a/apps/web/src/app/(app)/claw/components/changelog-data.ts +++ b/apps/web/src/app/(app)/claw/components/changelog-data.ts @@ -13,7 +13,7 @@ export const CHANGELOG_ENTRIES: ChangelogEntry[] = [ { date: '2026-05-27', description: - 'Saving openclaw.json from Edit Files now checks the installed OpenClaw configuration rules first. If validation fails, your edits are preserved and you can review the warning before choosing to save anyway.', + 'Saving openclaw.json from the file explorer in Settings now runs OpenClaw config validation first. If validation fails, your edits are preserved so you can review the warning before choosing Save anyway.', category: 'feature', deployHint: 'upgrade_required', }, From caf0f79d2f6a5fc891351893370ed7265b5b4be7 Mon Sep 17 00:00:00 2001 From: syn Date: Thu, 28 May 2026 09:33:24 -0500 Subject: [PATCH 5/5] fix(kiloclaw): serialize config file mutations --- .specs/kiloclaw-controller.md | 3 +- .../src/openclaw-config-validation.test.ts | 37 ++++++++- .../src/openclaw-config-validation.ts | 59 +++++++++----- .../controller/src/routes/files.test.ts | 78 +++++++++++++++++++ .../kiloclaw/controller/src/routes/files.ts | 74 +++++++++++++----- 5 files changed, 210 insertions(+), 41 deletions(-) diff --git a/.specs/kiloclaw-controller.md b/.specs/kiloclaw-controller.md index f0c3fc2c0f..019d966459 100644 --- a/.specs/kiloclaw-controller.md +++ b/.specs/kiloclaw-controller.md @@ -741,7 +741,8 @@ running. When forwarding, it MUST authenticate to gog with 5. Validation-warning responses MAY return bounded diagnostics derived while validating the authenticated instance's configuration, including substituted values; those diagnostics MUST be returned only to authenticated config-management clients and MUST NOT be logged. Responses and logs MUST NOT expose staging paths or unrestricted subprocess output. 6. Validation-aware writes MUST remain usable when the gateway process is unavailable after controller routes are registered. 7. Controllers implementing this behavior MUST advertise `files.write-openclaw-config`; clients MUST NOT infer this behavior solely from controller CalVer. -8. This validation reports OpenClaw configuration validity. It MUST NOT be represented as proof that runtime SecretRefs or optional environment substitutions resolve successfully. +8. Controllers implementing this behavior MUST serialize any remaining generic `POST /_kilo/files/write` mutation of `openclaw.json` with validation-aware writes and other controller-owned config mutations so legacy clients cannot interleave config commits. +9. This validation reports OpenClaw configuration validity. It MUST NOT be represented as proof that runtime SecretRefs or optional environment substitutions resolve successfully. #### Doctor (bearer token) diff --git a/services/kiloclaw/controller/src/openclaw-config-validation.test.ts b/services/kiloclaw/controller/src/openclaw-config-validation.test.ts index 9dba79fabf..3e17284cc5 100644 --- a/services/kiloclaw/controller/src/openclaw-config-validation.test.ts +++ b/services/kiloclaw/controller/src/openclaw-config-validation.test.ts @@ -81,7 +81,9 @@ describe('validateOpenclawConfigCandidate', () => { await expect(validateOpenclawConfigCandidate('{}', CONFIG_PATH, deps)).resolves.toEqual({ valid: false, reason: 'validation-unavailable', - issues: [{ path: '', message: 'OpenClaw configuration validation could not be started.' }], + issues: [ + { path: '', message: 'There is not enough disk space to validate this configuration.' }, + ], }); expect(log).toHaveBeenCalledWith( '[openclaw-config-validation] Validation failed unexpectedly:', @@ -92,6 +94,25 @@ describe('validateOpenclawConfigCandidate', () => { log.mockRestore(); }); + it.each([ + ['EACCES', 'OpenClaw cannot access the temporary validation file.'], + ['EPERM', 'OpenClaw cannot access the temporary validation file.'], + ['EEXIST', 'Configuration validation is already in progress. Try saving again.'], + ])('returns actionable messages for staging error %s', async (code, message) => { + const deps = createDeps(JSON.stringify({ valid: true })); + const log = vi.spyOn(console, 'error').mockImplementation(() => undefined); + vi.mocked(deps.writeCandidate).mockImplementation(() => { + throw Object.assign(new Error('staging failed'), { code }); + }); + + await expect(validateOpenclawConfigCandidate('{}', CONFIG_PATH, deps)).resolves.toEqual({ + valid: false, + reason: 'validation-unavailable', + issues: [{ path: '', message }], + }); + log.mockRestore(); + }); + it('redacts staging filenames from invalid diagnostics', async () => { const deps = createDeps( JSON.stringify({ @@ -139,6 +160,20 @@ describe('validateOpenclawConfigCandidate', () => { expect(deps.runValidation).not.toHaveBeenCalled(); }); + it('inspects deeply nested candidates without recursive traversal', async () => { + const deps = createDeps(JSON.stringify({ valid: true })); + const depth = 10_000; + const candidate = `${'{"nested":'.repeat(depth)}{"$include":"./openclaw.json"}${'}'.repeat(depth)}`; + + await expect( + validateOpenclawConfigCandidate(candidate, CONFIG_PATH, deps) + ).resolves.toMatchObject({ + valid: false, + reason: 'validation-unavailable', + }); + expect(deps.writeCandidate).not.toHaveBeenCalled(); + }); + it('warns without staging JSON5 candidates, including escaped self-targeting includes', async () => { for (const candidate of [ "{ agents: { $include: './openclaw.json' } }", diff --git a/services/kiloclaw/controller/src/openclaw-config-validation.ts b/services/kiloclaw/controller/src/openclaw-config-validation.ts index c8b7dd1b6c..255d90fbca 100644 --- a/services/kiloclaw/controller/src/openclaw-config-validation.ts +++ b/services/kiloclaw/controller/src/openclaw-config-validation.ts @@ -143,29 +143,46 @@ function unavailableIssue(message: string): OpenclawConfigValidationResult { }; } -function referencesTargetConfig(value: unknown, configPath: string): boolean { - if (Array.isArray(value)) { - return value.some(entry => referencesTargetConfig(entry, configPath)); - } - if (typeof value !== 'object' || value === null) { - return false; +function unexpectedValidationFailure(error: unknown): OpenclawConfigValidationResult { + switch (errorCode(error)) { + case 'ENOSPC': + return unavailableIssue('There is not enough disk space to validate this configuration.'); + case 'EACCES': + case 'EPERM': + return unavailableIssue('OpenClaw cannot access the temporary validation file.'); + case 'EEXIST': + return unavailableIssue('Configuration validation is already in progress. Try saving again.'); + default: + return unavailableIssue('OpenClaw configuration validation could not be started.'); } +} - for (const [key, nestedValue] of Object.entries(value)) { - if (key === '$include') { - const includes = Array.isArray(nestedValue) ? nestedValue : [nestedValue]; - if ( - includes.some( - include => - typeof include === 'string' && - path.resolve(path.dirname(configPath), include) === path.resolve(configPath) - ) - ) { - return true; - } +function referencesTargetConfig(value: unknown, configPath: string): boolean { + const pending: unknown[] = [value]; + while (pending.length > 0) { + const current = pending.pop(); + if (Array.isArray(current)) { + for (const entry of current) pending.push(entry); + continue; } - if (referencesTargetConfig(nestedValue, configPath)) { - return true; + if (typeof current !== 'object' || current === null) { + continue; + } + + for (const [key, nestedValue] of Object.entries(current)) { + if (key === '$include') { + const includes = Array.isArray(nestedValue) ? nestedValue : [nestedValue]; + if ( + includes.some( + include => + typeof include === 'string' && + path.resolve(path.dirname(configPath), include) === path.resolve(configPath) + ) + ) { + return true; + } + } + pending.push(nestedValue); } } return false; @@ -231,7 +248,7 @@ export async function validateOpenclawConfigCandidate( return unavailableIssue('OpenClaw could not validate this configuration.'); } catch (error) { console.error('[openclaw-config-validation] Validation failed unexpectedly:', errorCode(error)); - return unavailableIssue('OpenClaw configuration validation could not be started.'); + return unexpectedValidationFailure(error); } finally { try { deps.removeFile(stagePath); diff --git a/services/kiloclaw/controller/src/routes/files.test.ts b/services/kiloclaw/controller/src/routes/files.test.ts index febf92f01c..41dc6318f7 100644 --- a/services/kiloclaw/controller/src/routes/files.test.ts +++ b/services/kiloclaw/controller/src/routes/files.test.ts @@ -378,6 +378,56 @@ describe('file routes', () => { ); }); + it('serializes legacy openclaw writes through in-root aliases behind validation-aware writes', async () => { + vi.mocked(fs.existsSync).mockReturnValue(true); + vi.mocked(fs.lstatSync).mockReturnValue({ + isSymbolicLink: () => false, + isFile: () => true, + } as any); + vi.mocked(fs.realpathSync).mockImplementation(filePath => { + const resolvedPath = String(filePath); + return resolvedPath.includes('/alias/') ? `${ROOT}/openclaw.json` : resolvedPath; + }); + let resolveValidation: ((result: { valid: true }) => void) | undefined; + vi.mocked(validateOpenclawConfigCandidate).mockReturnValue( + new Promise(resolve => { + resolveValidation = resolve; + }) + ); + + const validatedWrite = app.request('/_kilo/files/write-openclaw-config', { + method: 'POST', + headers: authHeaders(), + body: JSON.stringify({ content: '{"validated":true}', mode: 'warn-before-write' }), + }); + await vi.waitFor(() => expect(validateOpenclawConfigCandidate).toHaveBeenCalledOnce()); + + const legacyWrite = app.request('/_kilo/files/write', { + method: 'POST', + headers: authHeaders(), + body: JSON.stringify({ path: 'alias/openclaw.json', content: '{"legacy":true}' }), + }); + await Promise.resolve(); + expect(atomicWrite).not.toHaveBeenCalled(); + + if (!resolveValidation) throw new Error('Validation request did not start'); + resolveValidation({ valid: true }); + await Promise.all([validatedWrite, legacyWrite]); + + expect(atomicWrite).toHaveBeenNthCalledWith( + 1, + `${ROOT}/openclaw.json`, + '{"validated":true}', + undefined, + { mode: 0o600 } + ); + expect(atomicWrite).toHaveBeenNthCalledWith( + 2, + `${ROOT}/alias/openclaw.json`, + '{"legacy":true}' + ); + }); + it('returns a conflict if openclaw.json disappears during ETag validation', async () => { vi.mocked(fs.existsSync).mockReturnValue(true); vi.mocked(fs.lstatSync).mockReturnValue({ @@ -439,6 +489,34 @@ describe('file routes', () => { expect(atomicWrite).not.toHaveBeenCalled(); }); + it('logs only safe error metadata when a config backup fails', async () => { + vi.mocked(fs.existsSync).mockReturnValue(true); + vi.mocked(fs.lstatSync).mockReturnValue({ + isSymbolicLink: () => false, + isFile: () => true, + } as any); + const warn = vi.spyOn(console, 'warn').mockImplementation(() => undefined); + vi.mocked(backupFile).mockImplementation(() => { + throw Object.assign(new Error('backup failed at /root/.openclaw/openclaw.json'), { + code: 'EACCES', + }); + }); + + const res = await app.request('/_kilo/files/write-openclaw-config', { + method: 'POST', + headers: authHeaders(), + body: JSON.stringify({ content: '{"gateway":{}}', mode: 'allow-invalid' }), + }); + + expect(res.status).toBe(200); + expect(warn).toHaveBeenCalledWith( + '[files] Failed to create backup, proceeding with write:', + 'EACCES' + ); + expect(warn.mock.calls.flat().join(' ')).not.toContain('/root/.openclaw/openclaw.json'); + warn.mockRestore(); + }); + it('allows an explicit invalid openclaw config override', async () => { vi.mocked(fs.existsSync).mockReturnValue(true); vi.mocked(fs.lstatSync).mockReturnValue({ diff --git a/services/kiloclaw/controller/src/routes/files.ts b/services/kiloclaw/controller/src/routes/files.ts index 627be6ff67..a151351798 100644 --- a/services/kiloclaw/controller/src/routes/files.ts +++ b/services/kiloclaw/controller/src/routes/files.ts @@ -68,6 +68,28 @@ type OpenclawWorkspacePreparedFile = { type OpenclawImportFailure = z.infer; +function errorCode(error: unknown): string { + if ( + typeof error === 'object' && + error !== null && + 'code' in error && + typeof error.code === 'string' + ) { + return error.code; + } + return 'unknown'; +} + +function resolvesToOpenclawConfig(resolvedPath: string, rootDir: string): boolean { + const configPath = path.resolve(rootDir, 'openclaw.json'); + if (resolvedPath === configPath) return true; + try { + return fs.realpathSync(resolvedPath) === fs.realpathSync(configPath); + } catch { + return false; + } +} + type OpenclawWorkspaceImportValidation = | { ok: true; @@ -720,25 +742,41 @@ export function registerFileRoutes(app: Hono, expectedToken: string, rootDir: st ); } - if (body.etag) { - const currentContent = fs.readFileSync(result, 'utf-8'); - if (body.etag !== computeEtag(currentContent)) { - return c.json({ code: 'file_etag_conflict', error: 'File was modified externally' }, 409); + const writeFile = () => { + if (body.etag) { + try { + const currentContent = fs.readFileSync(result, 'utf-8'); + if (body.etag !== computeEtag(currentContent)) { + return c.json( + { code: 'file_etag_conflict', error: 'File was modified externally' }, + 409 + ); + } + } catch { + return c.json({ code: 'file_etag_conflict', error: 'File was modified externally' }, 409); + } + } + + try { + backupFile(result, rootDir); + } catch (error) { + console.warn('[files] Failed to create backup, proceeding with write:', errorCode(error)); } - } + try { + atomicWrite(result, body.content); + } catch (err) { + console.error('[files] atomicWrite failed:', err); + return c.json({ error: 'Failed to write file' }, 500); + } + return c.json({ etag: computeEtag(body.content) }); + }; - try { - backupFile(result, rootDir); - } catch (err) { - console.warn('[files] Failed to create backup, proceeding with write:', err); - } - try { - atomicWrite(result, body.content); - } catch (err) { - console.error('[files] atomicWrite failed:', err); - return c.json({ error: 'Failed to write file' }, 500); + if (resolvesToOpenclawConfig(result, rootDir)) { + return serializeAgentConfigMutation(async () => writeFile(), { + configPath: path.resolve(rootDir, 'openclaw.json'), + }); } - return c.json({ etag: computeEtag(body.content) }); + return writeFile(); }); const WriteOpenclawConfigBodySchema = z.object({ @@ -797,8 +835,8 @@ export function registerFileRoutes(app: Hono, expectedToken: string, rootDir: st try { backupFile(result, rootDir); - } catch (err) { - console.warn('[files] Failed to create backup, proceeding with write:', err); + } catch (error) { + console.warn('[files] Failed to create backup, proceeding with write:', errorCode(error)); } try { atomicWrite(result, body.content, undefined, { mode: 0o600 });