fix(migrations): sweep already-parked subblock values; align picker 403

- an earlier version of this migration renamed retired fields into _removed_*
  keys instead of deleting them, so deployed workflows still hold those values;
  they match no oldId, so a dedicated sweep clears them for every block type
- the picker now treats a Snowflake 403 like a 401: it means a network policy
  or a disabled SQL API, which the credential validator already reports as a
  credential problem rather than a bad request

Author: waleedlatif1

apps/sim/app/api/tools/snowflake/objects/route.ts

@@ -141,21 +141,25 @@ export const POST = withRouteHandler(async (request: NextRequest) => {
       signal: AbortSignal.any([request.signal, AbortSignal.timeout(SELECTOR_FETCH_TIMEOUT_MS)]),
     })
 
-    // A rejected or expired token must tell the picker to reconnect rather than
-    // read as a Snowflake outage.
-    if (response.status === 401) {
+    // A rejected credential must tell the picker to reconnect rather than read
+    // as an outage or a bad request. 403 belongs here alongside 401: Snowflake
+    // uses it for a network-policy rejection and for a disabled SQL API, which
+    // is why the credential validator also treats it as a credential problem.
+    if (response.status === 401 || response.status === 403) {
       logger.warn('Snowflake rejected the stored credential', { credentialId: credential, kind })
       return NextResponse.json(
         {
-          error: 'Snowflake rejected this credential. Reconnect it and try again.',
+          error:
+            'Snowflake rejected this credential. Check that it has not expired and that a network policy allows Sim to reach the account, then reconnect it.',
           authRequired: true,
         },
         { status: 401 }
       )
     }
 
-    // A 4xx names something wrong with the request (missing object, privilege
-    // gap); only a 5xx or a malformed body is a gateway failure.
+    // A remaining 4xx names something wrong with the request itself (unknown
+    // object, malformed statement); only a 5xx or an unreadable body is a
+    // gateway failure.
     upstreamStatus = response.status
     const result = await readSnowflakeResult(response)
     // A metadata-only statement completes synchronously; a 202 means Snowflake

apps/sim/lib/workflows/migrations/subblock-migrations.test.ts

@@ -113,6 +113,46 @@ describe('migrateSubblockIds', () => {
     })
   })
 
+  /**
+   * An earlier version of this migration renamed retired fields into a
+   * `_removed_*` key instead of deleting them, so deployed workflows still hold
+   * those values. They match no `oldId`, so only a dedicated sweep clears them.
+   */
+  it('drops values parked by an earlier run of the migration', () => {
+    const input: Record<string, BlockState> = {
+      b1: makeBlock({
+        type: 'rippling',
+        subBlocks: {
+          _removed_email: { id: '_removed_email', type: 'short-input', value: 'ada@example.com' },
+          _removed_firstName: { id: '_removed_firstName', type: 'short-input', value: 'Ada' },
+          // Not in rippling's rename map, so it must survive the sweep untouched.
+          credential: { id: 'credential', type: 'oauth-input', value: 'cred-1' },
+        },
+      }),
+      // A block type with no rename map at all must still be swept.
+      b2: makeBlock({
+        type: 'snowflake',
+        subBlocks: {
+          _removed_apiKey: {
+            id: '_removed_apiKey',
+            type: 'short-input',
+            value: 'super-secret-pat',
+          },
+        },
+      }),
+    }
+
+    const { blocks, migrated } = migrateSubblockIds(input)
+
+    expect(migrated).toBe(true)
+    expect(blocks.b1.subBlocks._removed_email).toBeUndefined()
+    expect(blocks.b1.subBlocks._removed_firstName).toBeUndefined()
+    expect(blocks.b1.subBlocks.credential?.value).toBe('cred-1')
+    expect(blocks.b2.subBlocks._removed_apiKey).toBeUndefined()
+    expect(JSON.stringify(blocks)).not.toContain('super-secret-pat')
+    expect(JSON.stringify(blocks)).not.toContain('ada@example.com')
+  })
+
   describe('knowledge block', () => {
     it('should rename knowledgeBaseId to knowledgeBaseSelector', () => {
       const input: Record<string, BlockState> = {

apps/sim/lib/workflows/migrations/subblock-migrations.ts

@@ -212,6 +212,29 @@ function migrateBlockSubblockIds(
   return { subBlocks: result, migrated: true }
 }
 
+/**
+ * Drops any `_removed_*` subblock left behind by an earlier version of this
+ * migration, which renamed retired fields into a dead key instead of deleting
+ * them. Those keys are unreachable from the block config, so secret scrubbing —
+ * which walks the config — can never clear them, and a parked token or PII
+ * would otherwise survive in state, exports, and templates indefinitely.
+ *
+ * Runs for every block, not just those with a rename map: the parked keys no
+ * longer appear in any `SUBBLOCK_ID_MIGRATIONS` entry as an `oldId`, so nothing
+ * else would ever look at them.
+ */
+function dropParkedSubblocks(subBlocks: Record<string, BlockState['subBlocks'][string]>): {
+  subBlocks: Record<string, BlockState['subBlocks'][string]>
+  dropped: boolean
+} {
+  const parked = Object.keys(subBlocks).filter((id) => id.startsWith(REMOVED_SUBBLOCK_ID_PREFIX))
+  if (parked.length === 0) return { subBlocks, dropped: false }
+
+  const result = { ...subBlocks }
+  for (const id of parked) delete result[id]
+  return { subBlocks: result, dropped: true }
+}
+
 /**
  * Applies subblock-ID migrations to every block in a workflow.
  * Returns a new blocks record with migrated subBlocks where needed.
@@ -233,11 +256,19 @@ export function migrateSubblockIds(blocks: Record<string, BlockState>): {
     const renamed = renames
       ? migrateBlockSubblockIds(block.type, block.subBlocks, renames)
       : { subBlocks: block.subBlocks, migrated: false }
-    const renamedBlock = renamed.migrated ? { ...block, subBlocks: renamed.subBlocks } : block
+    const purged = dropParkedSubblocks(renamed.subBlocks)
+    const changedSubBlocks = renamed.migrated || purged.dropped
+    const renamedBlock = changedSubBlocks ? { ...block, subBlocks: purged.subBlocks } : block
     const sanitized = sanitizeMalformedSubBlocks(renamedBlock)
-    const blockMigrated = renamed.migrated || sanitized.changed
+    const blockMigrated = changedSubBlocks || sanitized.changed
 
     if (blockMigrated) {
+      if (purged.dropped) {
+        logger.info('Dropped parked subblock values left by an earlier migration', {
+          blockId: block.id,
+          blockType: block.type,
+        })
+      }
       if (renamed.migrated) {
         logger.info('Migrated legacy subblock IDs', {
           blockId: block.id,