Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: add sensitive env var clearing and e2e tests for session launcher #2701

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
54 changes: 54 additions & 0 deletions e2e/session-launcher.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,54 @@
import {
createSession,
deleteSession,
loginAsUser,
navigateTo,
} from './test-util';
import { test, expect } from '@playwright/test';

test.describe('NEO Sessions Launcher', () => {
test.beforeEach(async ({ page }) => {
await loginAsUser(page);
});

const sessionName = 'e2e-test-session';
test('User can create session in NEO', async ({ page }) => {
await createSession(page, sessionName);
await deleteSession(page, sessionName);
});

test('Sensitive environment variables are cleared when the browser is reloaded.', async ({
page,
}) => {
await navigateTo(page, 'session/start');
await page
.getByRole('button', { name: '2 Environments & Resource' })
.click();
await page
.getByRole('button', { name: 'plus Add environment variables' })
.click();
await page.getByPlaceholder('Variable').fill('abc');
await page.getByPlaceholder('Variable').press('Tab');
await page.getByPlaceholder('Value').fill('123');
await page
.getByRole('button', { name: 'plus Add environment variables' })
.click();
await page.locator('#envvars_1_variable').fill('password');
await page.locator('#envvars_1_variable').press('Tab');
await page.locator('#envvars_1_value').fill('hello');
await page
.getByRole('button', { name: 'plus Add environment variables' })
.click();
await page.locator('#envvars_2_variable').fill('api_key');
await page.locator('#envvars_2_variable').press('Tab');
await page.locator('#envvars_2_value').fill('secret');
await page.waitForTimeout(1000); // Wait for the form state to be saved as query param.
await page.reload();
await expect(
page.locator('#envvars_1_value_help').getByText('Please enter a value.'),
).toBeVisible();
await expect(
page.locator('#envvars_2_value_help').getByText('Please enter a value.'),
).toBeVisible();
});
});
59 changes: 0 additions & 59 deletions e2e/session.test.ts

This file was deleted.

35 changes: 35 additions & 0 deletions react/src/components/EnvVarFormList.test.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
// EnvVarFormList.test.tsx
import { sanitizeSensitiveEnv } from './EnvVarFormList';

describe('emptySensitiveEnv', () => {
it('should empty the value of sensitive environment variables', () => {
const envs = [
{ variable: 'SECRET_KEY', value: '12345' },
{ variable: 'API_KEY', value: 'abcdef' },
{ variable: 'NON_SENSITIVE', value: 'value' },
];

const result = sanitizeSensitiveEnv(envs);

expect(result).toEqual([
{ variable: 'SECRET_KEY', value: '' },
{ variable: 'API_KEY', value: '' },
{ variable: 'NON_SENSITIVE', value: 'value' },
]);
});

it('should not change non-sensitive environment variables', () => {
const envs = [{ variable: 'NON_SENSITIVE', value: 'value' }];
const result = sanitizeSensitiveEnv(envs);

expect(result).toEqual([{ variable: 'NON_SENSITIVE', value: 'value' }]);
});

it('should handle an empty array', () => {
const envs: any[] = [];

const result = sanitizeSensitiveEnv(envs);

expect(result).toEqual([]);
});
});
36 changes: 36 additions & 0 deletions react/src/components/EnvVarFormList.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -137,4 +137,40 @@ const EnvVarFormList: React.FC<EnvVarFormListProps> = ({
);
};

const sensitivePatterns = [
/AUTH/i,
/ACCESS/i,
/SECRET/i,
/_KEY/i,
/PASSWORD/i,
/PASSWD/i,
/PWD/i,
/TOKEN/i,
/PRIVATE/i,
/CREDENTIAL/i,
/JWT/i,
/KEYPAIR/i,
/CERTIFICATE/i,
/SSH/i,
/ENCRYPT/i,
/SIGNATURE/i,
/SALT/i,
/PIN/i,
/PASSPHRASE/i,
/OAUTH/i,
];

export function isSensitiveEnv(key: string) {
return sensitivePatterns.some((pattern) => pattern.test(key));
}

export function sanitizeSensitiveEnv(envs: EnvVarFormListValue[]) {
return envs.map((env) => {
if (env && isSensitiveEnv(env.variable)) {
return { ...env, value: '' };
}
return env;
});
}

export default EnvVarFormList;
1 change: 1 addition & 0 deletions react/src/components/VFolderTableFormItem.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ interface VFolderTableFormItemProps extends Omit<FormItemProps, 'name'> {
export interface VFolderTableFormValues {
mounts: string[];
vfoldersAliasMap: AliasMap;
autoMountedFolderNames?: string[];
}

const VFolderTableFormItem: React.FC<VFolderTableFormItemProps> = ({
Expand Down
20 changes: 14 additions & 6 deletions react/src/pages/SessionLauncherPage.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ import BAIIntervalText from '../components/BAIIntervalText';
import DatePickerISO from '../components/DatePickerISO';
import DoubleTag from '../components/DoubleTag';
import EnvVarFormList, {
sanitizeSensitiveEnv,
EnvVarFormListValue,
} from '../components/EnvVarFormList';
import Flex from '../components/Flex';
Expand Down Expand Up @@ -236,15 +237,22 @@ const SessionLauncherPage = () => {
// console.log('syncFormToURLWithDebounce', form.getFieldsValue());
// To sync the latest form values to URL,
// 'trailing' is set to true, and get the form values here."
const currentValue = form.getFieldsValue();
setQuery(
{
// formValues: form.getFieldsValue(),
formValues: _.omit(
form.getFieldsValue(),
['environments.image'],
['environments.customizedTag'],
['autoMountedFolderNames'],
['owner'],
formValues: _.extend(
_.omit(
form.getFieldsValue(),
['environments.image'],
['environments.customizedTag'],
['autoMountedFolderNames'],
['owner'],
['envvars'],
),
{
envvars: sanitizeSensitiveEnv(currentValue.envvars),
},
),
},
'replaceIn',
Expand Down
Loading