Просмотр исходного кода

Add unit tests for anonymizeQueryParams function and improve parameter handling

Yuki Takei 9 месяцев назад
Родитель
Сommit
37fafe2a02

+ 37 - 0
apps/app/src/features/opentelemetry/server/anonymization/utils/anonymize-query-params.spec.ts

@@ -0,0 +1,37 @@
+import { describe, it, expect } from 'vitest';
+
+import { anonymizeQueryParams } from './anonymize-query-params';
+
+describe('anonymizeQueryParams', () => {
+  /* eslint-disable max-len */
+  it.each`
+    description                       | target                                                                 | paramNames         | expected
+    ${'no matching parameters'}       | ${'/_api/v3/test?other=value&another=test'}                            | ${['nonexistent']} | ${'/_api/v3/test?other=value&another=test'}
+    ${'single string parameter'}      | ${'/_api/v3/search?q=sensitive-query'}                                 | ${['q']}           | ${'/_api/v3/search?q=%5BANONYMIZED%5D'}
+    ${'array-style parameters'}       | ${'/_api/v3/page/test?paths[]=/user/john&paths[]=/user/jane'}          | ${['paths']}       | ${'/_api/v3/page/test?paths%5B%5D=%5BANONYMIZED%5D'}
+    ${'JSON array format'}            | ${'/_api/v3/test?paths=["/user/john","/user/jane"]'}                   | ${['paths']}       | ${'/_api/v3/test?paths=%5B%22%5BANONYMIZED%5D%22%5D'}
+    ${'multiple parameters'}          | ${'/_api/v3/test?q=secret&path=/user/john&other=keep'}                 | ${['q', 'path']}   | ${'/_api/v3/test?q=%5BANONYMIZED%5D&path=%5BANONYMIZED%5D&other=keep'}
+    ${'empty parameter value'}        | ${'/_api/v3/test?q=&other=value'}                                      | ${['q']}           | ${'/_api/v3/test?q=&other=value'}
+    ${'parameter without value'}      | ${'/_api/v3/test?q&other=value'}                                       | ${['q']}           | ${'/_api/v3/test?q&other=value'}
+    ${'mixed array and single'}       | ${'/_api/v3/test?q=search&paths[]=/user/john&paths[]=/user/jane'}      | ${['q', 'paths']}  | ${'/_api/v3/test?q=%5BANONYMIZED%5D&paths%5B%5D=%5BANONYMIZED%5D'}
+    ${'malformed JSON array'}         | ${'/_api/v3/test?paths=["/user/john"'}                                 | ${['paths']}       | ${'/_api/v3/test?paths=%5BANONYMIZED%5D'}
+    ${'empty JSON array'}             | ${'/_api/v3/test?paths=[]'}                                            | ${['paths']}       | ${'/_api/v3/test?paths=%5BANONYMIZED%5D'}
+    ${'single item JSON array'}       | ${'/_api/v3/test?paths=["/user/john"]'}                                | ${['paths']}       | ${'/_api/v3/test?paths=%5B%22%5BANONYMIZED%5D%22%5D'}
+    ${'URL with no query params'}     | ${'/_api/v3/test'}                                                     | ${['q']}           | ${'/_api/v3/test'}
+    ${'complex path with encoding'}   | ${'/_api/v3/test?path=%2Fuser%2Fjohn%20doe'}                           | ${['path']}        | ${'/_api/v3/test?path=%5BANONYMIZED%5D'}
+  `('should handle $description', ({ target, paramNames, expected }) => {
+  /* eslint-enable max-len */
+    const result = anonymizeQueryParams(target, paramNames);
+    expect(result).toBe(expected);
+  });
+
+  it.each`
+    description                    | target                         | paramNames    | expected
+    ${'invalid URL format'}       | ${'not-a-valid-url'}           | ${['q']}      | ${'not-a-valid-url'}
+    ${'empty string target'}      | ${''}                          | ${['q']}      | ${''}
+    ${'empty paramNames array'}   | ${'/_api/v3/test?q=secret'}    | ${[]}         | ${'/_api/v3/test?q=secret'}
+  `('should handle edge cases: $description', ({ target, paramNames, expected }) => {
+    const result = anonymizeQueryParams(target, paramNames);
+    expect(result).toBe(expected);
+  });
+});

+ 35 - 15
apps/app/src/features/opentelemetry/server/anonymization/utils/anonymize-query-params.ts

@@ -1,6 +1,19 @@
 import { diag } from '@opentelemetry/api';
 
-const logger = diag.createComponentLogger({ namespace: 'growi:anonymization:query-params' });
+const logger = diag.createComponentLogger({ namespace: 'growi:anonymization:anonymize-query-params' });
+
+/**
+ * Try to parse JSON array, return null if invalid
+ */
+function tryParseJsonArray(value: string): unknown[] | null {
+  try {
+    const parsed = JSON.parse(value);
+    return Array.isArray(parsed) ? parsed : null;
+  }
+  catch {
+    return null;
+  }
+}
 
 /**
  * Anonymize specific query parameters in HTTP target URL
@@ -12,28 +25,35 @@ export function anonymizeQueryParams(target: string, paramNames: string[]): stri
   try {
     const url = new URL(target, 'http://localhost');
     const searchParams = new URLSearchParams(url.search);
-    let hasAnonymization = false;
+    let hasChange = false;
 
-    // Anonymize each specified parameter if it exists
     for (const paramName of paramNames) {
+      // Handle regular parameter (including JSON arrays)
       if (searchParams.has(paramName)) {
-        const originalValue = searchParams.get(paramName);
-        if (originalValue) {
-          // Replace the parameter content with [ANONYMIZED] but keep the parameter structure
-          searchParams.set(paramName, '[ANONYMIZED]');
-          hasAnonymization = true;
-          logger.debug(`Anonymized query parameter '${paramName}': original length=${originalValue.length}`);
+        const value = searchParams.get(paramName);
+        if (value) {
+          let replacement = '[ANONYMIZED]';
+          if (value.startsWith('[') && value.endsWith(']')) {
+            const jsonArray = tryParseJsonArray(value);
+            if (jsonArray && jsonArray.length > 0) {
+              replacement = '["[ANONYMIZED]"]';
+            }
+          }
+          searchParams.set(paramName, replacement);
+          hasChange = true;
         }
       }
-    }
 
-    if (!hasAnonymization) {
-      return target; // No changes needed
+      // Handle array-style parameters (paramName[])
+      const arrayParam = `${paramName}[]`;
+      if (searchParams.has(arrayParam)) {
+        searchParams.delete(arrayParam);
+        searchParams.set(arrayParam, '[ANONYMIZED]');
+        hasChange = true;
+      }
     }
 
-    // Reconstruct the target URL with anonymized parameters
-    url.search = searchParams.toString();
-    return url.pathname + url.search;
+    return hasChange ? `${url.pathname}?${searchParams.toString()}` : target;
   }
   catch (error) {
     logger.warn(`Failed to anonymize query parameters [${paramNames.join(', ')}]: ${error}`);