refactor(fetch): pf-4589 sanitize error messages, logs

src/__tests__/__snapshots__/options.defaults.test.ts.snap

@@ -178,6 +178,7 @@ exports[`options defaults should return specific properties: defaults 1`] = `
     "allowBinary": false,
     "maxSizeBytes": 5242880,
     "preflightHead": false,
+    "sanitizeMessages": true,
     "timeoutMs": 15000,
   },
 }

src/__tests__/__snapshots__/server.fetch.test.ts.snap

@@ -3,7 +3,7 @@
 exports[`setFetch should block outbound fetch and log warning in test mode when baseUrl is not configured: warning 1`] = `
 [
   [
-    "Network access blocked for https://patternfly.org: server is in 'test' mode and 'modeOptions.test.baseUrl' is not configured.",
+    "Network access blocked for https://patternfly.org/: server is in 'test' mode and 'modeOptions.test.baseUrl' is not configured.",
     "Configure 'modeOptions.test.baseUrl' to allow outbound fetches.",
   ],
 ]

src/__tests__/logger.helpers.test.ts

@@ -27,6 +27,53 @@ describe('sanitizeHeaderContent', () => {
       input: 'Authorization: Bearer abc123',
       options: { redacted: '<MASKED>' },
       expected: 'Authorization: Bearer <MASKED>'
+    },
+    {
+      description: 'api_key assignment',
+      input: 'api_key=xyz123',
+      expected: 'api_key=[REDACTED]'
+    },
+    {
+      description: 'api-key header',
+      input: 'api-key: xyz123',
+      expected: 'api-key: [REDACTED]'
+    },
+    {
+      description: 'token assignment',
+      input: 'token=abc456',
+      expected: 'token=[REDACTED]'
+    },
+    {
+      description: 'key assignment',
+      input: 'key=secret123',
+      expected: 'key=[REDACTED]'
+    },
+    {
+      description: 'auth assignment',
+      input: 'auth=secret123',
+      expected: 'auth=[REDACTED]'
+    },
+    {
+      description: 'authorization assignment',
+      input: 'authorization=secret123',
+      expected: 'authorization=[REDACTED]'
+    },
+    {
+      description: 'api-key without space after colon',
+      input: 'api-key:xyz123',
+      expected: 'api-key:[REDACTED]'
+    },
+    {
+      description: 'custom secretParamRegExp option',
+      input: 'custom_identifier=xyz123',
+      options: { secretParamRegExp: /custom_identifier/i },
+      expected: 'custom_identifier=[REDACTED]'
+    },
+    {
+      description: 'custom secretParamRegExp option with multiple search params',
+      input: 'custom_identifier=xyz123!&lorem=ipsum',
+      options: { secretParamRegExp: /custom_identifier/i },
+      expected: 'custom_identifier=[REDACTED]&lorem=ipsum'
     }
   ])('should sanitize header content, $description', ({ input, options, expected }) => {
     expect(sanitizeHeaderContent(input, options as any)).toBe(expected);
@@ -55,6 +102,31 @@ describe('sanitizeUrlContent', () => {
       input: 'Fetch https://example.com/api?access_token=xyz now',
       expected: 'Fetch https://example.com/api?access_token=%5BREDACTED%5D now'
     },
+    {
+      description: 'sanitize a URL with a parenthesis in its path',
+      input: 'Fetch https://example.com/a)path?token=abc123 now',
+      expected: 'Fetch https://example.com/a)path?token=%5BREDACTED%5D now'
+    },
+    {
+      description: 'sanitize a token ending in a parenthesis',
+      input: 'Fetch (https://example.com/docs?token=abc123)',
+      expected: 'Fetch (https://example.com/docs?token=%5BREDACTED%5D'
+    },
+    {
+      description: 'sanitize a token ending in nested parentheses',
+      input: 'Fetch ((https://example.com/a)path?token=abc123))',
+      expected: 'Fetch ((https://example.com/a)path?token=%5BREDACTED%5D'
+    },
+    {
+      description: 'sanitize a token ending in square brackets',
+      input: 'Fetch [https://example.com/docs?token=abc123]',
+      expected: 'Fetch [https://example.com/docs?token=%5BREDACTED%5D'
+    },
+    {
+      description: 'sanitize a token ending in an angle bracket',
+      input: 'Fetch <https://example.com/docs?token=abc123>',
+      expected: 'Fetch <https://example.com/docs?token=%5BREDACTED%5D'
+    },
     {
       description: 'non-string input',
       input: null,
@@ -66,7 +138,8 @@ describe('sanitizeUrlContent', () => {
       options: { redacted: '<MASKED>' },
       expected: 'https://example.com/docs?token=%3CMASKED%3E'
     }
-  ])('should sanitize url content, $description', ({ input, options, expected }) => {
+  ])('should sanitize url content while maintaining delimiter-like characters, $description', ({ input, options, expected }) => {
+    // IMPORTANT: Delimiter-like characters are intentionally kept intact and not stripped from URLs.
     expect(sanitizeUrlContent(input as any, options as any)).toBe(expected);
   });
 });
@@ -136,6 +209,11 @@ describe('sanitizeMessage', () => {
       description: 'non-string serializable object',
       input: { auth: 'Authorization: Bearer abc123' },
       expected: '{"auth":"Authorization: Bearer [REDACTED]"}'
+    },
+    {
+      description: 'sanitize non-URL api_key and token assignments',
+      input: 'Failed upstream: api_key=xyz123 and token=abc456',
+      expected: 'Failed upstream: api_key=[REDACTED] and token=[REDACTED]'
     }
   ])('should sanitize message content, $description', ({ input, expected }) => {
     expect(sanitizeMessage(input)).toContain(expected);

src/__tests__/server.fetch.test.ts

@@ -82,6 +82,79 @@ describe('decodeStream', () => {
   });
 });
 
+describe('FetchError', () => {
+  it.each([
+    {
+      description: 'sanitize an error message',
+      options: {
+        message: 'Failed to fetch https://patternfly.org/x?private_token=abc123',
+        sanitize: true
+      },
+      expected: {
+        message: expect.stringContaining('private_token=%5BREDACTED%5D')
+      }
+    },
+    {
+      description: 'not sanitize an error message',
+      options: {
+        message: 'Failed to fetch https://patternfly.org/x?private_token=abc123',
+        sanitize: false
+      },
+      expected: {
+        message: expect.stringContaining('private_token=abc123')
+      }
+    },
+    {
+      description: 'sanitize statusText',
+      options: {
+        message: 'Failed to fetch',
+        statusText: 'Unauthorized: private_token=abc123',
+        sanitize: true
+      },
+      expected: {
+        statusText: expect.stringContaining('private_token=[REDACTED]')
+      }
+    },
+    {
+      description: 'sanitize statusText with api_key and token',
+      options: {
+        message: 'Failed to fetch',
+        statusText: 'Unauthorized: api_key=xyz123 and token=abc456',
+        sanitize: true
+      },
+      expected: {
+        statusText: 'Unauthorized: api_key=[REDACTED] and token=[REDACTED]'
+      }
+    },
+    {
+      description: 'not sanitize statusText',
+      options: {
+        message: 'Failed to fetch',
+        statusText: 'Unauthorized: private_token=abc123',
+        sanitize: false
+      },
+      expected: {
+        statusText: 'Unauthorized: private_token=abc123'
+      }
+    }
+  ])('should $description', ({ options, expected }) => {
+    const err = new FetchError(options);
+
+    expect(err).toMatchObject(expected);
+  });
+
+  it('should preserve raw cause object identity', () => {
+    const cause = new Error('upstream fail private_token=abc123');
+    const err = new FetchError({
+      message: 'wrapper',
+      cause,
+      sanitize: true
+    });
+
+    expect(err.cause).toBe(cause);
+  });
+});
+
 describe('parsePayload', () => {
   it('should parse valid JSON', async () => {
     const payload = { kind: 'text' as const, mimeType: 'application/json', text: '{"key": "value"}' };
@@ -252,6 +325,60 @@ describe('setFetch', () => {
     expect(postPhase).toBe('error');
   });
 
+  it.each([
+    {
+      description: 'sanitize URL and statusText secrets',
+      url: 'https://patternfly.org/secure?private_token=abc123&view=full',
+      statusText: 'Unauthorized: access_token=secret123',
+      expected: {
+        message: expect.stringContaining('private_token=%5BREDACTED%5D'),
+        statusText: expect.stringContaining('access_token=[REDACTED]')
+      },
+      unexpected: ['private_token=abc123', 'access_token=secret123']
+    },
+    {
+      description: 'preserve the status delimiter after sanitizing a request URL',
+      url: 'https://patternfly.org/secure?access_token=abc123&view=full',
+      statusText: 'Unauthorized',
+      expected: {
+        message: expect.stringContaining('https://patternfly.org/secure?access_token=%5BREDACTED%5D&view=full: 401 Unauthorized'),
+        statusText: expect.stringContaining('Unauthorized')
+      },
+      unexpected: ['access_token=abc123']
+    },
+    {
+      description: 'sanitize a URL secret in statusText with a parenthesis in its path',
+      url: 'https://patternfly.org/start',
+      statusText: 'Unauthorized: https://patternfly.org/a)path?token=abc123',
+      expected: {
+        message: expect.stringContaining('https://patternfly.org/a)path?token=%5BREDACTED%5D'),
+        statusText: expect.stringContaining('https://patternfly.org/a)path?token=%5BREDACTED%5D')
+      },
+      unexpected: ['token=abc123']
+    }
+  ])('should $description for a non-ok response', async ({ url, statusText, expected, unexpected }) => {
+    (global.fetch as jest.Mock).mockResolvedValue({
+      ok: false,
+      status: 401,
+      statusText,
+      headers: {
+        get: () => null
+      }
+    });
+
+    const { get } = setFetch();
+    const error = await get(url).catch(reason => reason);
+
+    expect(error).toMatchObject(expected);
+
+    unexpected.forEach(value => {
+      expect(error).toMatchObject({
+        message: expect.not.stringContaining(value),
+        statusText: expect.not.stringContaining(value)
+      });
+    });
+  });
+
   it('should check content-length against maxSizeBytes', async () => {
     const mockResponse = {
       ok: true,
@@ -276,7 +403,7 @@ describe('setFetch', () => {
     expect((status() as any).phase).toBe('error');
   });
 
-  it('should reject redirected URLs that are not whitelisted', async () => {
+  it('should reject redirected URLs that are not allowlisted', async () => {
     const mockResponse = {
       ok: true,
       url: 'https://untrusted.com/data',
@@ -292,6 +419,54 @@ describe('setFetch', () => {
     expect(mockResponse.body.cancel).toHaveBeenCalled();
   });
 
+  it('should sanitize secrets in pre-request allowlist errors', async () => {
+    const options = {
+      ...getOptions(),
+      whitelist: {
+        urls: ['https://allowed.patternfly.org']
+      }
+    };
+    const { get } = setFetch(options as any);
+
+    await expect(get('https://blocked.patternfly.org/path?private_token=abc123')).rejects.toMatchObject({
+      message: expect.stringContaining('private_token=%5BREDACTED%5D')
+    });
+
+    await expect(get('https://blocked.patternfly.org/path?private_token=abc123')).rejects.toMatchObject({
+      message: expect.not.stringContaining('private_token=abc123')
+    });
+  });
+
+  it('should sanitize secrets in post-redirect allowlist errors', async () => {
+    const options = {
+      ...getOptions(),
+      whitelist: {
+        urls: ['https://www.patternfly.org']
+      }
+    };
+
+    const mockResponse = {
+      ok: true,
+      status: 200,
+      statusText: 'OK',
+      url: 'https://error.patternfly.org/landing?access_token=secret123',
+      headers: { get: () => 'text/plain' },
+      body: new ReadableStream({ start(controller) { controller.close(); } })
+    };
+
+    (global.fetch as jest.Mock).mockResolvedValue(mockResponse);
+    const { get } = setFetch(options as any);
+
+    // Leverage an allowlist url to check post-redirect
+    await expect(get('https://www.patternfly.org/start')).rejects.toMatchObject({
+      message: expect.stringContaining('access_token=%5BREDACTED%5D')
+    });
+
+    await expect(get('https://www.patternfly.org/start')).rejects.toMatchObject({
+      message: expect.not.stringContaining('access_token=secret123')
+    });
+  });
+
   it('should de-duplicate concurrent requests to the same URL', async () => {
     (global.fetch as jest.Mock).mockReturnValue(new Promise(() => {})); // Hang
     const { get } = setFetch();
@@ -401,6 +576,30 @@ describe('setFetch', () => {
     warnSpy.mockRestore();
   });
 
+  it('should preserve the warning URL when message sanitization is disabled', async () => {
+    const warnSpy = jest.spyOn(log, 'warn').mockImplementation(() => {});
+    const url = 'https://patternfly.org/path?private_token=abc123';
+    const options = getOptions();
+    const fetchOptions = {
+      ...options,
+      mode: 'test' as const,
+      modeOptions: {},
+      xhrFetch: {
+        ...options.xhrFetch,
+        sanitizeMessages: false
+      }
+    };
+    const { get } = setFetch(fetchOptions as any);
+
+    await expect(get(url)).rejects.toThrow(FetchError);
+    expect(warnSpy).toHaveBeenCalledWith(
+      `Network access blocked for ${url}: server is in 'test' mode and 'modeOptions.test.baseUrl' is not configured.`,
+      `Configure 'modeOptions.test.baseUrl' to allow outbound fetches.`
+    );
+
+    warnSpy.mockRestore();
+  });
+
   it('should allow matching fixture URL and reject external URLs in test mode with baseUrl configured', async () => {
     const warnSpy = jest.spyOn(log, 'warn').mockImplementation(() => {});
 
@@ -450,4 +649,34 @@ describe('setFetch', () => {
 
     warnSpy.mockRestore();
   });
+
+  it.each([
+    {
+      description: 'sanitize unknown errors by default',
+      sanitizeMessages: undefined,
+      expectedMessage: 'lorem ipsum private_token=[REDACTED]'
+    },
+    {
+      description: 'retain unknown errors when sanitization is disabled',
+      sanitizeMessages: false,
+      expectedMessage: 'lorem ipsum private_token=abc123'
+    }
+  ])('should $description', async ({ sanitizeMessages, expectedMessage }) => {
+    (global.fetch as jest.Mock).mockRejectedValue(new Error('lorem ipsum private_token=abc123'));
+    const options = getOptions();
+    const fetchOptions = sanitizeMessages === undefined
+      ? undefined
+      : {
+        ...options,
+        xhrFetch: {
+          ...options.xhrFetch,
+          sanitizeMessages
+        }
+      };
+    const { get } = setFetch(fetchOptions);
+
+    await expect(get('https://patternfly.org/data')).rejects.toMatchObject({
+      message: expect.stringContaining(expectedMessage)
+    });
+  });
 });