From 5506a08308bb1c192d947015e0aa9ad072ccb95b Mon Sep 17 00:00:00 2001 From: Elliott Hamai Date: Thu, 21 Dec 2023 10:26:02 -0800 Subject: [PATCH] Making sure not to redact any response properties for ARG queries within a BATCH request --- .../requestRules/armBatchResponseRule.test.ts | 54 +++++++++++++++++-- .../requestRules/armBatchResponseRule.ts | 24 +++++++-- 2 files changed, 68 insertions(+), 10 deletions(-) diff --git a/src/sanitizer/requestRules/armBatchResponseRule.test.ts b/src/sanitizer/requestRules/armBatchResponseRule.test.ts index a2898d0..f725e76 100644 --- a/src/sanitizer/requestRules/armBatchResponseRule.test.ts +++ b/src/sanitizer/requestRules/armBatchResponseRule.test.ts @@ -52,6 +52,16 @@ 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: {} }] }; @@ -59,7 +69,8 @@ test('armBatchResponseRule sanitize ', () => { 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); }) \ No newline at end of file diff --git a/src/sanitizer/requestRules/armBatchResponseRule.ts b/src/sanitizer/requestRules/armBatchResponseRule.ts index 3eb0546..80fa023 100644 --- 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; }