fix: fix isByosVault flag for secrets created with forceDB if readonly_vault is used (#3103)

Author: brojd

platform/backend/src/agents/chatops/chatops-manager.ts

@@ -199,9 +199,22 @@ export class ChatOpsManager {
     await this.seedConfigFromEnvVars();
 
     // Load configs from DB (the single source of truth)
+    // Errors are caught individually so a single broken config doesn't prevent other providers from initializing
     const [msTeamsConfig, slackConfig] = await Promise.all([
-      ChatOpsConfigModel.getMsTeamsConfig(),
-      ChatOpsConfigModel.getSlackConfig(),
+      ChatOpsConfigModel.getMsTeamsConfig().catch((error) => {
+        logger.error(
+          { error: error instanceof Error ? error.message : String(error) },
+          "[ChatOps] Failed to load MS Teams config, skipping",
+        );
+        return null;
+      }),
+      ChatOpsConfigModel.getSlackConfig().catch((error) => {
+        logger.error(
+          { error: error instanceof Error ? error.message : String(error) },
+          "[ChatOps] Failed to load Slack config, skipping",
+        );
+        return null;
+      }),
     ]);
 
     // Create providers with their config

platform/backend/src/models/chatops-config.ts

@@ -72,21 +72,13 @@ class ChatOpsConfigModel {
   }
 
   private async getConfig<T>(secretName: string): Promise<T | null> {
-    try {
-      const secretRow = await SecretModel.findByName(secretName);
-      if (!secretRow) return null;
+    const secretRow = await SecretModel.findByName(secretName);
+    if (!secretRow) return null;
 
-      const secret = await secretManager().getSecret(secretRow.id);
-      if (!secret?.secret) return null;
+    const secret = await secretManager().getSecret(secretRow.id);
+    if (!secret?.secret) return null;
 
-      return secret.secret as unknown as T;
-    } catch (error) {
-      logger.error(
-        { error: error instanceof Error ? error.message : String(error) },
-        `ChatOpsConfigModel: failed to read config "${secretName}"`,
-      );
-      return null;
-    }
+    return secret.secret as unknown as T;
   }
 
   private async saveConfig(

platform/backend/src/secrets-manager/readonly-vault.ee.ts

@@ -1,4 +1,4 @@
-import { SecretsManagerType } from "@shared";
+import { isVaultReference, SecretsManagerType } from "@shared";
 import logger from "@/logging";
 import SecretModel from "@/models/secret";
 import {
@@ -115,8 +115,27 @@ export default class ReadonlyVaultSecretManager
       return dbRecord;
     }
 
+    const secretValues = dbRecord.secret as Record<string, unknown>;
+
+    // Self-heal corrupted records:
+    // We had a bug where we were setting isByosVault to true during updateSecret for records created with forceDB=true introduced here:
+    // https://github.com/archestra-ai/archestra/pull/1694/changes#diff-02457052fc1fed9d616aebeae6f7e0d984575808d3b218ccfd851300dcf7793aR426
+    // In this case, if we detect that the secret values have is_byos_vault=true but are not vault references,
+    // we need to fix the flag and return the DB record as-is.
+    const hasAnyVaultReference = Object.values(secretValues).some(
+      (v) => typeof v === "string" && isVaultReference(v),
+    );
+    if (!hasAnyVaultReference) {
+      logger.warn(
+        { secretId },
+        "BYOSVaultSecretManager.getSecret: isByosVault=true but no vault references found, fixing corrupted flag",
+      );
+      await SecretModel.update(secretId, { isByosVault: false });
+      return { ...dbRecord, isByosVault: false };
+    }
+
     // All values in secret field are vault references (path#key format)
-    const vaultReferences = dbRecord.secret as Record<string, string>;
+    const vaultReferences = secretValues;
     if (Object.keys(vaultReferences).length === 0) {
       return dbRecord;
     }
@@ -195,8 +214,8 @@ export default class ReadonlyVaultSecretManager
     }
 
     return await SecretModel.update(secretId, {
+      // Do not modify isByosVault — that flag is set at creation time and shouldn't be changed during update.
       secret: _secretValue,
-      isByosVault: true,
     });
   }
 
@@ -219,7 +238,7 @@ export default class ReadonlyVaultSecretManager
    * Groups by path to minimize Vault API calls.
    */
   private async resolveVaultReferences(
-    references: Record<string, string>,
+    references: Record<string, unknown>,
   ): Promise<SecretValue> {
     const resolved: SecretValue = {};
 

platform/backend/src/secrets-manager/secrets-manager.test.ts

@@ -28,6 +28,9 @@ vi.mock("node-vault", () => {
 describe("SecretsManager", async () => {
   // biome-ignore lint/style/noRestrictedImports: dynamic import
   const VaultSecretManager = (await import("./vault.ee")).default;
+  // biome-ignore lint/style/noRestrictedImports: dynamic import
+  const ReadonlyVaultSecretManager = (await import("./readonly-vault.ee"))
+    .default;
 
   describe("getSecretsManagerTypeBasedOnEnvVars", () => {
     const originalEnv = process.env;
@@ -1133,4 +1136,94 @@ describe("SecretsManager", async () => {
       });
     });
   });
+
+  describe("ReadonlyVaultSecretManager", () => {
+    const vaultConfig = {
+      address: "http://localhost:8200",
+      authMethod: "token" as const,
+      kvVersion: "2" as const,
+      token: "dev-root-token",
+      secretPath: "secret/data/archestra",
+      k8sTokenPath: "/var/run/secrets/kubernetes.io/serviceaccount/token",
+      k8sMountPoint: "kubernetes",
+      awsMountPoint: "aws",
+      awsRegion: "us-east-1",
+      awsStsEndpoint: "https://sts.amazonaws.com",
+    };
+
+    beforeEach(() => {
+      vi.clearAllMocks();
+    });
+
+    describe("getSecret - self-healing corrupted isByosVault", () => {
+      test("should fix isByosVault flag when secret values are not vault references", async () => {
+        const byosManager = new ReadonlyVaultSecretManager(vaultConfig);
+
+        // Create a forceDB secret (isByosVault=false, plain DB values)
+        const created = await byosManager.createSecret(
+          { apiKey: "sk-test-key-123" },
+          "test-oauth-secret",
+          true,
+        );
+        expect(created.isByosVault).toBe(false);
+
+        // Simulate the bug: manually set isByosVault to true
+        await SecretModel.update(created.id, {
+          secret: { apiKey: "sk-test-key-123" },
+          isByosVault: true,
+        });
+
+        // getSecret should detect the mismatch and self-heal
+        const result = await byosManager.getSecret(created.id);
+        expect(result).not.toBeNull();
+        expect(result?.isByosVault).toBe(false);
+        expect(result?.secret).toEqual({ apiKey: "sk-test-key-123" });
+
+        // Verify the DB was fixed
+        const dbRecord = await SecretModel.findById(created.id);
+        expect(dbRecord?.isByosVault).toBe(false);
+      });
+
+      test("should not modify isByosVault when secret values are valid vault references", async () => {
+        const byosManager = new ReadonlyVaultSecretManager(vaultConfig);
+
+        // Create a proper BYOS secret with vault references
+        const created = await byosManager.createSecret(
+          { apiKey: "secret/data/my-keys#api_key" },
+          "test-vault-ref",
+        );
+        expect(created.isByosVault).toBe(true);
+
+        // getSecret should try to resolve vault references (will fail since no real vault)
+        // but should NOT flip isByosVault to false
+        await expect(byosManager.getSecret(created.id)).rejects.toThrow();
+
+        // Verify the flag was NOT changed
+        const dbRecord = await SecretModel.findById(created.id);
+        expect(dbRecord?.isByosVault).toBe(true);
+      });
+    });
+
+    describe("updateSecret - preserves isByosVault", () => {
+      test("should not modify isByosVault flag on update", async () => {
+        const byosManager = new ReadonlyVaultSecretManager(vaultConfig);
+
+        // Create a forceDB secret (like OAuth tokens)
+        const created = await byosManager.createSecret(
+          { access_token: "oauth-token-123", refresh_token: "refresh-456" },
+          "test-oauth",
+          true,
+        );
+        expect(created.isByosVault).toBe(false);
+
+        // Update with new plain values (like token refresh)
+        const updated = await byosManager.updateSecret(created.id, {
+          access_token: "new-oauth-token",
+          refresh_token: "new-refresh",
+        });
+        expect(updated).not.toBeNull();
+        expect(updated?.isByosVault).toBe(false);
+      });
+    });
+  });
 });

platform/backend/src/types/secret.ts

@@ -27,7 +27,7 @@ export const InsertSecretSchema = createInsertSchema(schema.secretsTable, {
   updatedAt: true,
 });
 export const UpdateSecretSchema = createUpdateSchema(schema.secretsTable, {
-  secret: SecretValueSchema,
+  secret: SecretValueSchema.optional(),
 }).omit({
   id: true,
   createdAt: true,

platform/frontend/src/lib/chatops.query.ts

@@ -12,8 +12,12 @@ export function useChatOpsStatus() {
   return useQuery({
     queryKey: ["chatops", "status"],
     queryFn: async () => {
-      const response = await archestraApiSdk.getChatOpsStatus();
-      return response.data?.providers || [];
+      const { data, error } = await archestraApiSdk.getChatOpsStatus();
+      if (error) {
+        handleApiError(error);
+        return null;
+      }
+      return data?.providers || [];
     },
   });
 }