Skip to content

Commit

Permalink
chore(ui): check Op code for literal credentials
Browse files Browse the repository at this point in the history
  • Loading branch information
jamie-rasmussen committed Oct 29, 2024
1 parent abbe2e0 commit 4b5e18d
Show file tree
Hide file tree
Showing 3 changed files with 50 additions and 3 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ import Box from '@mui/material/Box';
import {Loading} from '@wandb/weave/components/Loading';
import React, {FC} from 'react';

import {sanitizeString} from '../../../../util/sanitizeSecrets';
import {Alert} from '../../../Alert';
import {useWFHooks} from '../Browse3/pages/wfReactInterface/context';

Expand Down Expand Up @@ -54,14 +55,15 @@ export const Browse2OpDefCode: FC<{uri: string; maxRowsInView?: number}> = ({
);
}

const detectedLanguage = detectLanguage(uri, text.result ?? '');
const sanitized = sanitizeString(text.result ?? '');
const detectedLanguage = detectLanguage(uri, sanitized);

const inner = (
<Editor
height={'100%'}
defaultLanguage={detectedLanguage}
loading={text.loading}
value={text.result ?? ''}
value={sanitized}
options={{
readOnly: true,
minimap: {enabled: false},
Expand All @@ -71,7 +73,7 @@ export const Browse2OpDefCode: FC<{uri: string; maxRowsInView?: number}> = ({
/>
);
if (maxRowsInView) {
const totalLines = text.result?.split('\n').length ?? 0;
const totalLines = sanitized.split('\n').length ?? 0;
const showLines = Math.min(totalLines, maxRowsInView);
const lineHeight = 18;
const padding = 20;
Expand Down
28 changes: 28 additions & 0 deletions weave-js/src/util/sanitizeSecrets.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
import {sanitizeString} from './sanitizeSecrets';

describe('sanitizeString', () => {
test('does not change clean string', () => {
expect(sanitizeString('foo')).toEqual('foo');
});
test('does not change non-literal value', () => {
expect(sanitizeString('"api_key": api_key')).toEqual('"api_key": api_key');
});
test('does sanitize literal value - double quotes', () => {
expect(sanitizeString('"api_key": "abc"')).toEqual(
'<Redacted: string contains api_key pattern>'
);
});
test('does sanitize literal value - single quotes', () => {
expect(sanitizeString('"api_key": \'abc\'')).toEqual(
'<Redacted: string contains api_key pattern>'
);
});
test('does sanitize known bad keys', () => {
expect(sanitizeString('"auth_headers": \'abc\'')).toEqual(
'<Redacted: string contains auth_headers pattern>'
);
expect(sanitizeString('"Authorization": \'abc\'')).toEqual(
'<Redacted: string contains Authorization pattern>'
);
});
});
17 changes: 17 additions & 0 deletions weave-js/src/util/sanitizeSecrets.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
const BAD_KEYS = ['api_key', 'auth_headers', 'Authorization'];

// Check for literal API key values in a string.
// Not comprehensive, but meant as a stopgap.
export const sanitizeString = (str: string): string => {
for (const badKey of BAD_KEYS) {
// Don't fire on non-literal key value, e.g. "api_key": api_key is OK
const regex = new RegExp(`['"]${badKey}['"]:\\s+['"]`, 'i');
if (regex.test(str)) {
// Note: This is replacing the entire string which seemed like
// the more cautious approach, we could also consider only
// redacting the literal value.
return `<Redacted: string contains ${badKey} pattern>`;
}
}
return str;
};

0 comments on commit 4b5e18d

Please sign in to comment.