fix: remove unsafe array secret restoration and preserve 409 conflict codes

Remove index-based array walking in walkAndRestore() which silently
swapped secrets between entries when users reordered arrays. Placeholders
in array entries are now stripped instead of restored.

Wire up UpstreamApiError so the tRPC error formatter surfaces
upstreamCode to clients. The config editor now checks for
config_etag_conflict specifically, so non-etag 409s (e.g. "Instance not
provisioned") show the actual error instead of a misleading reload prompt.

Author: evanjacobson

src/app/(app)/claw/components/OpenclawConfigEditor.tsx

@@ -225,7 +225,10 @@ export function OpenclawConfigEditor({
                   onOpenChange(false);
                 },
                 onError: err => {
-                  if (err.data?.code === 'CONFLICT') {
+                  if (
+                    err.data?.code === 'CONFLICT' &&
+                    err.data?.upstreamCode === 'config_etag_conflict'
+                  ) {
                     void refetch();
                     toast.error(
                       'Config was modified externally — click "Reload latest" to sync, then re-apply your changes'

src/lib/kiloclaw/config-redaction.test.ts

@@ -295,14 +295,22 @@ describe('array handling', () => {
     expect(providers[1].name).toBe('anthropic');
   });
 
-  it('restores secrets inside array entries from current config', () => {
+  it('strips placeholders in array entries on restore (no index-based matching)', () => {
     const redacted = redactOpenclawConfig(CONFIG_WITH_ARRAYS);
     const merged = restoreRedactedSecrets(redacted, CONFIG_WITH_ARRAYS);
 
-    expect(merged).toEqual(CONFIG_WITH_ARRAYS);
+    // Array secrets are NOT restored — placeholders are stripped to avoid
+    // position-dependent mismatches when users reorder entries.
+    const providers = (merged.models as Record<string, unknown>).providers as Array<
+      Record<string, unknown>
+    >;
+    expect(providers[0].apiKey).toBeUndefined();
+    expect(providers[0].name).toBe('openai');
+    expect(providers[1].apiKey).toBeUndefined();
+    expect(providers[1].name).toBe('anthropic');
   });
 
-  it('keeps new values in array entries when user changed a secret', () => {
+  it('keeps new values in array entries when user set a non-placeholder value', () => {
     const redacted = redactOpenclawConfig(CONFIG_WITH_ARRAYS);
     const providers = (redacted.models as Record<string, unknown>).providers as Array<
       Record<string, unknown>
@@ -314,10 +322,11 @@ describe('array handling', () => {
       Record<string, unknown>
     >;
     expect(mergedProviders[0].apiKey).toBe('sk-new-key');
-    expect(mergedProviders[1].apiKey).toBe('sk-secret-2');
+    // Placeholder in second entry is stripped
+    expect(mergedProviders[1].apiKey).toBeUndefined();
   });
 
-  it('strips unresolvable placeholders in array entries for new subtrees', () => {
+  it('strips placeholders in array entries even when currentConfig has data', () => {
     const userConfig = {
       models: {
         providers: [
@@ -335,7 +344,7 @@ describe('array handling', () => {
     expect(providers[0].baseUrl).toBe('https://example.com');
   });
 
-  it('redacts secrets in nested arrays (arrays inside array elements)', () => {
+  it('redacts secrets in nested arrays', () => {
     const config = {
       groups: [{ members: [{ apiKey: 'nested-secret', name: 'bot' }] }],
     };
@@ -345,10 +354,6 @@ describe('array handling', () => {
 
     expect(members[0].apiKey).toBe(REDACTED_PLACEHOLDER);
     expect(members[0].name).toBe('bot');
-
-    // round-trip
-    const restored = restoreRedactedSecrets(redacted, config);
-    expect(restored).toEqual(config);
   });
 
   it('handles mixed array contents (strings, numbers, objects, nulls)', () => {
@@ -374,21 +379,25 @@ describe('array handling', () => {
     expect(restored.providers).toEqual([]);
   });
 
-  it('handles deeply nested objects inside array elements', () => {
+  it('strips placeholders in deeply nested objects inside array elements', () => {
     const config = {
       list: [{ inner: { deep: { apiKey: 'deep-array-secret' } } }],
     };
     const redacted = redactOpenclawConfig(config);
     const list = redacted.list as Array<Record<string, unknown>>;
     const deep = (list[0].inner as Record<string, unknown>).deep as Record<string, unknown>;
-
     expect(deep.apiKey).toBe(REDACTED_PLACEHOLDER);
 
     const restored = restoreRedactedSecrets(redacted, config);
-    expect(restored).toEqual(config);
+    const restoredList = restored.list as Array<Record<string, unknown>>;
+    const restoredDeep = (restoredList[0].inner as Record<string, unknown>).deep as Record<
+      string,
+      unknown
+    >;
+    expect(restoredDeep.apiKey).toBeUndefined();
   });
 
-  it('strips placeholders when user array is longer than current array', () => {
+  it('strips all placeholders regardless of array length mismatch', () => {
     const userConfig = {
       providers: [
         { apiKey: REDACTED_PLACEHOLDER, name: 'existing' },
@@ -401,8 +410,7 @@ describe('array handling', () => {
 
     const merged = restoreRedactedSecrets(userConfig, currentConfig);
     const providers = merged.providers as Array<Record<string, unknown>>;
-    expect(providers[0].apiKey).toBe('real-key');
-    // Second element has no match in currentConfig — placeholder stripped
+    expect(providers[0].apiKey).toBeUndefined();
     expect(providers[1].apiKey).toBeUndefined();
     expect(providers[1].name).toBe('new-extra');
   });

src/lib/kiloclaw/config-redaction.ts

@@ -172,32 +172,12 @@ function walkAndRestore(obj: Record<string, unknown>, current: Record<string, un
         delete obj[key];
       }
     } else if (Array.isArray(value)) {
-      const currentArr = current[key];
-      if (Array.isArray(currentArr)) {
-        for (let i = 0; i < value.length; i++) {
-          const item = value[i];
-          const currentItem = currentArr[i];
-          if (typeof item === 'object' && item !== null && !Array.isArray(item)) {
-            if (
-              typeof currentItem === 'object' &&
-              currentItem !== null &&
-              !Array.isArray(currentItem)
-            ) {
-              walkAndRestore(
-                item as Record<string, unknown>,
-                currentItem as Record<string, unknown>
-              );
-            } else {
-              stripUnresolvablePlaceholders(item as Record<string, unknown>);
-            }
-          }
-        }
-      } else {
-        // Array doesn't exist in currentConfig — strip placeholders from all elements
-        for (const item of value) {
-          if (typeof item === 'object' && item !== null && !Array.isArray(item)) {
-            stripUnresolvablePlaceholders(item as Record<string, unknown>);
-          }
+      // Arrays are not walked for secret restoration — index-based matching
+      // is position-dependent and silently swaps secrets when users reorder
+      // entries. Strip any leftover placeholders so they don't leak through.
+      for (const item of value) {
+        if (typeof item === 'object' && item !== null && !Array.isArray(item)) {
+          stripUnresolvablePlaceholders(item as Record<string, unknown>);
         }
       }
     } else if (typeof value === 'object' && value !== null) {

src/lib/trpc/init.ts

@@ -30,6 +30,26 @@ export const createTRPCContext = async (): Promise<TRPCContext> => {
 // since it's not very descriptive.
 // For instance, the use of a t variable
 // is common in i18n libraries.
+/**
+ * Marker class used to attach an upstream API error code to a TRPCError so the
+ * error-formatter can surface it to the client in `err.data.upstreamCode`.
+ *
+ * Usage:
+ *   throw new TRPCError({
+ *     code: 'CONFLICT',
+ *     message: 'Config was modified',
+ *     cause: new UpstreamApiError('etag_mismatch'),
+ *   });
+ *
+ * The client then sees `err.data.upstreamCode === 'etag_mismatch'`.
+ */
+export class UpstreamApiError extends Error {
+  constructor(public readonly upstreamCode: string) {
+    super(upstreamCode);
+    this.name = 'UpstreamApiError';
+  }
+}
+
 const t = initTRPC.context<TRPCContext>().create({
   errorFormatter(opts) {
     const { shape, error } = opts;
@@ -41,6 +61,8 @@ const t = initTRPC.context<TRPCContext>().create({
           error.code === 'BAD_REQUEST' && error.cause instanceof z.ZodError
             ? z.flattenError(error.cause)
             : null,
+        upstreamCode:
+          error.cause instanceof UpstreamApiError ? error.cause.upstreamCode : undefined,
       },
     };
   },

src/routers/kiloclaw-router.ts

@@ -2,7 +2,7 @@ import 'server-only';
 
 import * as z from 'zod';
 import { TRPCError } from '@trpc/server';
-import { baseProcedure, createTRPCRouter } from '@/lib/trpc/init';
+import { baseProcedure, createTRPCRouter, UpstreamApiError } from '@/lib/trpc/init';
 import { generateApiToken, TOKEN_EXPIRY } from '@/lib/tokens';
 import { KiloClawInternalClient, KiloClawApiError } from '@/lib/kiloclaw/kiloclaw-internal-client';
 import { KiloClawUserClient } from '@/lib/kiloclaw/kiloclaw-user-client';
@@ -796,10 +796,11 @@ export const kiloclawRouter = createTRPCRouter({
         });
       }
       if (err instanceof KiloClawApiError && err.statusCode === 409) {
-        const { message } = getKiloClawApiErrorPayload(err);
+        const { message, code } = getKiloClawApiErrorPayload(err);
         throw new TRPCError({
           code: 'CONFLICT',
           message: message ?? 'Instance is not provisioned or not running',
+          cause: code ? new UpstreamApiError(code) : undefined,
         });
       }
       throw new TRPCError({
@@ -836,10 +837,11 @@ export const kiloclawRouter = createTRPCRouter({
           });
         }
         if (err instanceof KiloClawApiError && err.statusCode === 409) {
-          const { message } = getKiloClawApiErrorPayload(err);
+          const { message, code } = getKiloClawApiErrorPayload(err);
           throw new TRPCError({
             code: 'CONFLICT',
             message: message ?? 'Instance is not provisioned or not running',
+            cause: code ? new UpstreamApiError(code) : undefined,
           });
         }
         throw new TRPCError({