Making sure not to redact any response properties for ARG queries wit…

…hin a BATCH request
ehamai · Dec 21, 2023 · 5506a08 · 5506a08
1 parent cea87e8
commit 5506a08
Show file tree

Hide file tree

Showing 2 changed files with 68 additions and 10 deletions.
diff --git a/src/sanitizer/requestRules/armBatchResponseRule.test.ts b/src/sanitizer/requestRules/armBatchResponseRule.test.ts
@@ -52,14 +52,25 @@ test('armBatchResponseRule sanitize ', () => {
             httpMethod: 'POST',
             url: 'https://post-non-success-response',
             content: {}
+        },
+        {
+            httpMethod: 'POST',
+            url: 'https://management.azure.com/providers/microsoft.resourcegraph/resources?api-version=2023-12-01',
+            content: {}
+        },
+        {
+            httpMethod: 'POST',
+            url: '/providers/microsoft.resourcegraph/resources?api-version=2023-12-01',
+            content: {}
         }]
     };
 
     const uberBatchResponse = <UberBatchResponse>{
         responses: [{
             httpStatusCode: 200,
             content: {
-                stringProp: 'stringValue'
+                stringProp: 'stringValue',
+                secretProp: 'secretValue'
             }
         },
         {
@@ -82,6 +93,28 @@ test('armBatchResponseRule sanitize ', () => {
             content: {
                 stringProp: 'stringValue'
             }
+        },
+        {
+            httpStatusCode: 200,
+            content: {
+                stringProp: 'stringValue',
+                arrayProp: ['stringValue0'],
+                numProp: 1,
+                properties: {
+                    childStringProp: 'childStringValue'
+                }
+            }
+        },
+        {
+            httpStatusCode: 200,
+            content: {
+                stringProp: 'stringValue',
+                arrayProp: ['stringValue0'],
+                numProp: 1,
+                properties: {
+                    childStringProp: 'childStringValue'
+                }
+            }
         }]
     }
 
@@ -107,14 +140,25 @@ test('armBatchResponseRule sanitize ', () => {
     rule.sanitize(entry);
     const sanitizedUberResponse: UberBatchResponse = JSON.parse(entry.response.content.text);
 
-    expect(sanitizedUberResponse.responses[0].content.stringProp).toEqual('stringValue');
+    expect(sanitizedUberResponse.responses[0].content.stringProp).toEqual(uberBatchResponse.responses[0].content.stringProp);
+    expect(sanitizedUberResponse.responses[0].content.secretProp).toEqual(REDACTED);
 
     expect(sanitizedUberResponse.responses[1].content.stringProp).toEqual(REDACTED);
-    expect(sanitizedUberResponse.responses[1].content.arrayProp[0]).toEqual('stringValue0');
-    expect(sanitizedUberResponse.responses[1].content.numProp).toEqual(1);
+    expect(sanitizedUberResponse.responses[1].content.arrayProp[0]).toEqual(uberBatchResponse.responses[1].content.arrayProp[0]);
+    expect(sanitizedUberResponse.responses[1].content.numProp).toEqual(uberBatchResponse.responses[1].content.numProp);
     expect(sanitizedUberResponse.responses[1].content.properties.childStringProp).toEqual(REDACTED);
 
     expect(sanitizedUberResponse.responses[2].content).toEqual(REDACTED);
 
-    expect(sanitizedUberResponse.responses[3].content.stringProp).toEqual('stringValue');
+    expect(sanitizedUberResponse.responses[3].content.stringProp).toEqual(uberBatchResponse.responses[3].content.stringProp);
+
+    expect(sanitizedUberResponse.responses[4].content.stringProp).toEqual(uberBatchResponse.responses[4].content.stringProp);
+    expect(sanitizedUberResponse.responses[4].content.arrayProp[0]).toEqual(uberBatchResponse.responses[4].content.arrayProp[0]);
+    expect(sanitizedUberResponse.responses[4].content.numProp).toEqual(uberBatchResponse.responses[4].content.numProp);
+    expect(sanitizedUberResponse.responses[4].content.properties.childStringProp).toEqual(uberBatchResponse.responses[4].content.properties.childStringProp);
+
+    expect(sanitizedUberResponse.responses[5].content.stringProp).toEqual(uberBatchResponse.responses[5].content.stringProp);
+    expect(sanitizedUberResponse.responses[5].content.arrayProp[0]).toEqual(uberBatchResponse.responses[5].content.arrayProp[0]);
+    expect(sanitizedUberResponse.responses[5].content.numProp).toEqual(uberBatchResponse.responses[5].content.numProp);
+    expect(sanitizedUberResponse.responses[5].content.properties.childStringProp).toEqual(uberBatchResponse.responses[5].content.properties.childStringProp);
 })
diff --git a/src/sanitizer/requestRules/armBatchResponseRule.ts b/src/sanitizer/requestRules/armBatchResponseRule.ts
@@ -27,7 +27,7 @@ export class ArmBatchResponseRule implements SanitizationRule {
     isApplicable(requestEntry: Entry): boolean {
         const { request, response } = requestEntry;
 
-        if(!isBatchRequest(request)){
+        if (!isBatchRequest(request)) {
             return false;
         }
 
@@ -47,6 +47,8 @@ export class ArmBatchResponseRule implements SanitizationRule {
         const uberBatchRequest: UberBatchRequest = JSON.parse(request.postData.text);
         const uberBatchResponse: UberBatchResponse = JSON.parse(response.content.text);
 
+        // NOTE: Rethinking this, it would probably make more sense to just convert each individual batch request into a regular request
+        // and then run each of them through all of the sanitizer rules so that we wouldn't have to duplicate some of the logic here.
         for (let i = 0; i < uberBatchRequest.requests.length; i++) {
             const batchRequest = uberBatchRequest.requests[i];
             const batchResponse = uberBatchResponse.responses[i];
@@ -57,16 +59,28 @@ export class ArmBatchResponseRule implements SanitizationRule {
 
             if (typeof batchResponse.content === 'object') {
                 try {
-                    // If the request is a POST request, then we redact all JSON properties from the response
-                    // If it's not a POST request, then we just redact special keyword properties
-                    cleanProperties(batchResponse.content, this.getName(), batchRequest.httpMethod === 'POST' /* cleanAllProperties */);
+
+                    // Some batch requests only have relative path URLs
+                    let pathName = '';
+                    if (!batchRequest.url.startsWith('/')) {
+                        pathName = new URL(batchRequest.url).pathname.toLowerCase();
+                    } else {
+                        pathName = new URL(`https://temp.com${batchRequest.url.toLowerCase()}`).pathname.toLowerCase();
+                    }
+
+                    if (pathName !== '/providers/microsoft.resourcegraph/resources') {
+                        // If the request is a POST request, then we redact all JSON properties from the response
+                        // If it's not a POST request, then we just redact special keyword properties
+                        cleanProperties(batchResponse.content, this.getName(), batchRequest.httpMethod === 'POST' /* cleanAllProperties */);
+                    }
+
                 } catch (e) {
                     console.log('Failed to clean POST batch response. Redacting entire response');
 
                     // We err on the side of caution and redact the entire response.
                     batchResponse.content = REDACTED;
                 }
-            } else{
+            } else {
                 // If we can't parse the content then we redact the whole thing to be safe
                 batchResponse.content = REDACTED;
             }