cherrypick: two more runtime fixes (#3695)

* fix: more permissive type assertions (#3692) * fix: more permissive type assertions Fixes #3684 * fix: json serialization cycles (#3693) See microsoft/BotFramework-Composer#7854 for more details. * fix: limit serialized form of storage types These can end up in serialized JSON transcripts as well, so to limit the chance of including connection strings or other secrets we should restrict their serialized forms. Co-authored-by: Steven Gum <[email protected]> * fix: two typos (#3696) Co-authored-by: Steven Gum <[email protected]>
microsoft · May 18, 2021 · d2b0197 · d2b0197
1 parent 199aba1
commit d2b0197
Show file tree

Hide file tree

Showing 5 changed files with 40 additions and 23 deletions.
diff --git a/libraries/botbuilder-applicationinsights/src/applicationInsightsTelemetryClient.ts b/libraries/botbuilder-applicationinsights/src/applicationInsightsTelemetryClient.ts
@@ -114,6 +114,11 @@ export class ApplicationInsightsTelemetryClient implements BotTelemetryClient, B
         this.client.addTelemetryProcessor(addBotIdentifiers);
     }
 
+    // Protects against JSON.stringify cycles
+    private toJSON(): unknown {
+        return { name: 'ApplicationInsightsTelemetryClient' };
+    }
+
     /**
      * Provides access to the Application Insights configuration that is running here.
      * Allows developers to adjust the options, for example:

diff --git a/libraries/botbuilder-azure-blobs/package.json b/libraries/botbuilder-azure-blobs/package.json
@@ -32,7 +32,8 @@
     "botbuilder-core": "4.1.6",
     "botbuilder-stdlib": "4.1.6",
     "get-stream": "^6.0.0",
-    "p-map": "^4.0.0"
+    "p-map": "^4.0.0",
+    "runtypes": "~5.1.0"
   },
   "scripts": {
     "build": "tsc -b",

diff --git a/libraries/botbuilder-azure-blobs/src/blobsStorage.ts b/libraries/botbuilder-azure-blobs/src/blobsStorage.ts
@@ -1,11 +1,11 @@
 // Copyright (c) Microsoft Corporation.
 // Licensed under the MIT License.
 
+import * as t from 'runtypes';
 import getStream from 'get-stream';
 import pmap from 'p-map';
 import { ContainerClient, StoragePipelineOptions } from '@azure/storage-blob';
-import { Storage, StoreItems, assertStoreItems } from 'botbuilder-core';
-import { assert } from 'botbuilder-stdlib';
+import { Storage, StoreItems } from 'botbuilder-core';
 import { ignoreError, isStatusCodeError } from './ignoreError';
 import { sanitizeBlobKey } from './sanitizeBlobKey';
 
@@ -36,8 +36,7 @@ export class BlobsStorage implements Storage {
      * @param {BlobsStorageOptions} options Other options for BlobsStorage
      */
     constructor(connectionString: string, containerName: string, options?: BlobsStorageOptions) {
-        assert.string(connectionString, ['connectionString']);
-        assert.string(containerName, ['containerName']);
+        t.Record({ connectionString: t.String, containerName: t.String }).check({ connectionString, containerName });
 
         this._containerClient = new ContainerClient(connectionString, containerName, options?.storagePipelineOptions);
 
@@ -47,6 +46,11 @@ export class BlobsStorage implements Storage {
         }
     }
 
+    // Protects against JSON.stringify cycles
+    private toJSON(): unknown {
+        return { name: 'BlobsStorage' };
+    }
+
     private _initialize(): Promise<unknown> {
         if (!this._initializePromise) {
             this._initializePromise = this._containerClient.createIfNotExists();
@@ -61,7 +65,7 @@ export class BlobsStorage implements Storage {
      * @returns {Promise<StoreItems>} The fetched [StoreItems](xref:botbuilder-core.StoreItems)
      */
     async read(keys: string[]): Promise<StoreItems> {
-        assert.arrayOfString(keys, ['keys']);
+        t.Record({ keys: t.Array(t.String) }).check({ keys });
 
         await this._initialize();
 
@@ -105,7 +109,7 @@ export class BlobsStorage implements Storage {
      * @returns {Promise<void>} A promise representing the async operation
      */
     async write(changes: StoreItems): Promise<void> {
-        assertStoreItems(changes, ['changes']);
+        t.Dictionary(t.Unknown, t.String).withBrand('StoreItems').check(changes);
 
         await this._initialize();
 
@@ -132,7 +136,7 @@ export class BlobsStorage implements Storage {
      * @returns {Promise<void>} A promise representing the async operation
      */
     async delete(keys: string[]): Promise<void> {
-        assert.arrayOfString(keys, ['keys']);
+        t.Record({ keys: t.Array(t.String) }).check({ keys });
 
         await this._initialize();
 

diff --git a/libraries/botbuilder-azure-blobs/src/blobsTranscriptStore.ts b/libraries/botbuilder-azure-blobs/src/blobsTranscriptStore.ts
@@ -1,10 +1,10 @@
 // Copyright (c) Microsoft Corporation.
 // Licensed under the MIT License.
 
+import * as t from 'runtypes';
 import getStream from 'get-stream';
 import pmap from 'p-map';
-import { Activity, PagedResult, TranscriptInfo, TranscriptStore, assertActivity } from 'botbuilder-core';
-import { assert } from 'botbuilder-stdlib';
+import { Activity, PagedResult, TranscriptInfo, TranscriptStore } from 'botbuilder-core';
 import { maybeCast } from 'botbuilder-stdlib/lib/maybeCast';
 import { sanitizeBlobKey } from './sanitizeBlobKey';
 
@@ -32,14 +32,14 @@ function getConversationPrefix(channelId: string, conversationId: string): strin
     return sanitizeBlobKey(`${channelId}/${conversationId}`);
 }
 
+const DateT = t.Guard((val: unknown): val is Date => val instanceof Date, { name: 'Date' });
+
 // Formats an activity as a blob key
 function getBlobKey(activity: Activity): string {
-    assert.date(activity.timestamp, ['activity', 'timestamp']);
+    const { timestamp } = t.Record({ timestamp: DateT }).check(activity);
 
     return sanitizeBlobKey(
-        [activity.channelId, activity.conversation.id, `${formatTicks(activity.timestamp)}-${activity.id}.json`].join(
-            '/'
-        )
+        [activity.channelId, activity.conversation.id, `${formatTicks(timestamp)}-${activity.id}.json`].join('/')
     );
 }
 
@@ -71,15 +71,14 @@ export class BlobsTranscriptStore implements TranscriptStore {
     private _initializePromise?: Promise<unknown>;
 
     /**
-     * Constructs a BlobsStorage instance.
+     * Constructs a BlobsTranscriptStore instance.
      *
      * @param {string} connectionString Azure Blob Storage connection string
      * @param {string} containerName Azure Blob Storage container name
      * @param {BlobsTranscriptStoreOptions} options Other options for BlobsTranscriptStore
      */
     constructor(connectionString: string, containerName: string, options?: BlobsTranscriptStoreOptions) {
-        assert.string(connectionString, ['connectionString']);
-        assert.string(containerName, ['containerName']);
+        t.Record({ connectionString: t.String, containerName: t.String }).check({ connectionString, containerName });
 
         this._containerClient = new ContainerClient(connectionString, containerName, options?.storagePipelineOptions);
 
@@ -89,6 +88,11 @@ export class BlobsTranscriptStore implements TranscriptStore {
         }
     }
 
+    // Protects against JSON.stringify cycles
+    private toJSON(): unknown {
+        return { name: 'BlobsTranscriptStore' };
+    }
+
     private _initialize(): Promise<unknown> {
         if (!this._initializePromise) {
             this._initializePromise = this._containerClient.createIfNotExists();
@@ -112,8 +116,7 @@ export class BlobsTranscriptStore implements TranscriptStore {
         continuationToken?: string,
         startDate?: Date
     ): Promise<PagedResult<Activity>> {
-        assert.string(channelId, ['channelId']);
-        assert.string(conversationId, ['conversationId']);
+        t.Record({ channelId: t.String, conversationId: t.String }).check({ channelId, conversationId });
 
         await this._initialize();
 
@@ -182,7 +185,7 @@ export class BlobsTranscriptStore implements TranscriptStore {
      * [PagedResult](xref:botbuilder-core.PagedResult) of [Activity](xref:botbuilder-core.Activity) items
      */
     async listTranscripts(channelId: string, continuationToken?: string): Promise<PagedResult<TranscriptInfo>> {
-        assert.string(channelId, ['channelId']);
+        t.Record({ channelId: t.String }).check({ channelId });
 
         await this._initialize();
 
@@ -217,8 +220,7 @@ export class BlobsTranscriptStore implements TranscriptStore {
      * @returns {Promise<void>} A promise representing the async operation.
      */
     async deleteTranscript(channelId: string, conversationId: string): Promise<void> {
-        assert.string(channelId, ['channelId']);
-        assert.string(conversationId, ['conversationId']);
+        t.Record({ channelId: t.String, conversationId: t.String }).check({ channelId, conversationId });
 
         await this._initialize();
 
@@ -251,7 +253,7 @@ export class BlobsTranscriptStore implements TranscriptStore {
      * @returns {Promise<void>} A promise representing the async operation.
      */
     async logActivity(activity: Activity): Promise<void> {
-        assertActivity(activity, ['activity']);
+        t.Record({ activity: t.Dictionary(t.Unknown, t.String) }).check({ activity });
 
         await this._initialize();
 

diff --git a/libraries/botbuilder-azure/src/cosmosDbPartitionedStorage.ts b/libraries/botbuilder-azure/src/cosmosDbPartitionedStorage.ts
@@ -167,6 +167,11 @@ export class CosmosDbPartitionedStorage implements Storage {
         }
     }
 
+    // Protects against JSON.stringify cycles
+    private toJSON(): unknown {
+        return { name: 'CosmosDbPartitionedStorage' };
+    }
+
     /**
      * Read one or more items with matching keys from the Cosmos DB container.
      *