Skip to content

fix : [WTO-332] Sanitize sensitive information from URLs in terminal output #15053

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

Draft
wants to merge 2 commits into
base: main
Choose a base branch
from
Draft
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
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,9 @@ import { useTranslation } from 'react-i18next';
import { connect } from 'react-redux';
import { Dispatch } from 'redux';
import { impersonateStateToProps } from '@console/dynamic-plugin-sdk';
import { k8sGetResource } from '@console/dynamic-plugin-sdk/src/utils/k8s';
import { PodModel } from '@console/internal/models';
import { resourceURL, K8sKind } from '@console/internal/module/k8s';
import { resourceURL, K8sKind, PodKind } from '@console/internal/module/k8s';
import { WSFactory } from '@console/internal/module/ws-factory';
import { connectToFlags, WithFlagsProps } from '@console/internal/reducers/connectToFlags';
import { FLAGS, useTelemetry } from '@console/shared';
Expand Down Expand Up @@ -53,6 +54,54 @@ export type CloudShellExecProps = Props & DispatchProps & StateProps & WithFlags
const NO_SH =
'starting container process caused "exec: \\"sh\\": executable file not found in $PATH"';

export const sanitizeURL = (data: string, proxyUrl: any): string => {
if (!proxyUrl) {
return data;
}
const { httpProxy, httpsProxy } = proxyUrl;
let result = data;
if (httpProxy) {
result = data.replaceAll(httpProxy, httpProxy.replace(/(https?:\/\/)[^@]*@/g, '$1'));
}
if (httpsProxy) {
result = data.replaceAll(httpsProxy, httpsProxy.replace(/(https?:\/\/)[^@]*@/g, '$1'));
}
return result;
};

export const extractProxyEnvVarsFromWorkspacePod = async (podName: string, namespace: string) => {
try {
const pod = await k8sGetResource<PodKind>({
model: PodModel,
name: podName,
ns: namespace,
});

if (!pod?.spec?.containers) {
return null;
}
let httpProxyValue: string | null = null;
let httpsProxyValue: string | null = null;

for (const container of pod.spec.containers) {
for (const envVar of container.env ?? []) {
if (envVar.name === 'HTTP_PROXY') {
httpProxyValue = envVar.value;
} else if (envVar.name === 'HTTPS_PROXY') {
httpsProxyValue = envVar.value;
}
}
}

return {
httpProxy: httpProxyValue,
httpsProxy: httpsProxyValue,
};
} catch (error) {
return null;
}
};

const CloudShellExec: React.FC<CloudShellExecProps> = ({
workspaceName,
workspaceId,
Expand Down Expand Up @@ -100,6 +149,16 @@ const CloudShellExec: React.FC<CloudShellExecProps> = ({
const usedClient = flags[FLAGS.OPENSHIFT] ? 'oc' : 'kubectl';
const cmd = shcommand || ['sh', '-i', '-c', 'TERM=xterm sh'];
const subprotocols = (impersonate?.subprotocols || []).concat('base64.channel.k8s.io');
let webTerminalProxyUrls;

extractProxyEnvVarsFromWorkspacePod(podname, namespace)
.then((proxyUrls) => {
webTerminalProxyUrls = proxyUrls;
})
.catch((error) => {
webTerminalProxyUrls = { httpProxy: null, httpsProxy: null };
t('Error extracting proxy vars:', error);
});

const urlOpts = {
ns: namespace,
Expand Down Expand Up @@ -141,7 +200,8 @@ const CloudShellExec: React.FC<CloudShellExecProps> = ({
return;
}
}
const data = Base64.decode(msg.slice(1));
let data = Base64.decode(msg.slice(1));
data = sanitizeURL(data, webTerminalProxyUrls);
currentTerminal && currentTerminal.onDataReceived(data);
previous = data;
})
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,209 @@
import { k8sGetResource } from '@console/dynamic-plugin-sdk/src/utils/k8s';
import { extractProxyEnvVarsFromWorkspacePod, sanitizeURL } from '../CloudShellExec';

jest.mock('@console/dynamic-plugin-sdk/src/utils/k8s', () => ({
k8sGetResource: jest.fn(),
}));

describe('sanitizeURL', () => {
it('should remove username and password from URLs', () => {
// Given
const input = 'https://user:[email protected]/resource';
const expected = 'https://example.com/resource';
// When + Then
expect(sanitizeURL(input, { httpProxy: 'https://user:[email protected]/resource' })).toBe(
expected,
);
});

it('should leave URLs without credentials unchanged', () => {
// Given
const input = 'https://example.com/resource';
// When + Then
expect(sanitizeURL(input, { httpProxy: 'https://user:[email protected]/resource' })).toBe(input);
});

it('should handle multiple occurrences of credentials in a string', () => {
// Given
const input = 'https://user:[email protected]/path https://admin:[email protected]/resource';
const expected = 'https://site1.com/path https://admin:[email protected]/resource';
// When + Then
expect(sanitizeURL(input, { httpProxy: 'https://user:[email protected]/path' })).toBe(expected);
});

it('should handle missing username', () => {
// Given
const input = 'https://:[email protected]/resource';
const expected = 'https://example.com/resource';
// When + Then
expect(sanitizeURL(input, { httpProxy: 'https://:[email protected]/resource' })).toBe(
expected,
);
});

it('should handle missing password', () => {
// Given
const input = 'https://username:@example.com/resource';
const expected = 'https://example.com/resource';
// When + Then
expect(sanitizeURL(input, { httpProxy: 'https://username:@example.com/resource' })).toBe(
expected,
);
});

it('should handle missing username and password', () => {
// Given
const input = 'https://@some.host/foo/bar';
const expected = 'https://some.host/foo/bar';
// When + Then
expect(sanitizeURL(input, { httpProxy: 'https://@some.host/foo/bar' })).toBe(expected);
});

it('should handle IPv6 URLs with port numbers', () => {
// Given
const input = 'https://[::1]:456';
const expected = 'https://[::1]:456';
// When + Then
expect(sanitizeURL(input, { httpProxy: 'https://[::1]:456' })).toBe(expected);
});

it('should handle printenv output containing proxy URLs', () => {
// Given
const input =
'no_proxy=.cluster.local,.svc,.ap-south-1.compute.internal,localhost,172.30.0.1\n' +
'https_proxy=http://username:password@hostname:3128/\n' +
'NO_PROXY=.cluster.local,.svc,.ap-south-1.compute.internal,localhost,172.30.0.1\n' +
'HTTPS_PROXY=http://username:password@hostname:3128/\n' +
'HTTP_PROXY=http://username:password@hostname:3128/\n' +
'http_proxy=http://username:password@hostname:3128/';
const expected =
'no_proxy=.cluster.local,.svc,.ap-south-1.compute.internal,localhost,172.30.0.1\n' +
'https_proxy=http://hostname:3128/\n' +
'NO_PROXY=.cluster.local,.svc,.ap-south-1.compute.internal,localhost,172.30.0.1\n' +
'HTTPS_PROXY=http://hostname:3128/\n' +
'HTTP_PROXY=http://hostname:3128/\n' +
'http_proxy=http://hostname:3128/';
// When + Then
expect(sanitizeURL(input, { httpProxy: 'http://username:password@hostname:3128/' })).toBe(
expected,
);
});

it('should not modify non-URL text', () => {
// Given
const input = 'This is a test string without URLs.';
// When + Then
expect(sanitizeURL(input, { httpProxy: 'http://username:password@hostname:3128/' })).toBe(
input,
);
});
});

const k8sGetMocked = k8sGetResource as jest.Mock;

describe('extractProxyEnvVarsFromWorkspacePod', () => {
beforeEach(() => {
k8sGetMocked.mockClear();
});

afterEach(jest.resetAllMocks);

it('should extract both Proxy environment variables from Pod env if present', async () => {
// Given
k8sGetMocked.mockReturnValueOnce({
metadata: {
name: 'test-pod',
namespace: 'test-namespace',
},
spec: {
containers: [
{
env: [
{ name: 'HTTP_PROXY', value: 'https://username:password@hostname:3128/' },
{ name: 'HTTPS_PROXY', value: 'https://username:password@hostname:3128/' },
],
},
],
},
});
// When
const result = await extractProxyEnvVarsFromWorkspacePod('test-pod', 'test-namespace');
// Then
expect(result).toEqual({
httpProxy: 'https://username:password@hostname:3128/',
httpsProxy: 'https://username:password@hostname:3128/',
});
});

it('should return empty object if Proxy environment variables absent', async () => {
// Given
k8sGetMocked.mockReturnValueOnce({
metadata: {
name: 'test-pod',
namespace: 'test-namespace',
},
spec: {
containers: [
{
env: [{ name: 'ENV_VAR_NAME', value: 'myval' }],
},
],
},
});
// When
const result = await extractProxyEnvVarsFromWorkspacePod('test-pod', 'test-namespace');
// Then
expect(result).toEqual({
httpProxy: null,
httpsProxy: null,
});
});

it('should return object with only HTTP_PROXY if Proxy environment variables present', async () => {
// Given
k8sGetMocked.mockReturnValueOnce({
metadata: {
name: 'test-pod',
namespace: 'test-namespace',
},
spec: {
containers: [
{
env: [{ name: 'HTTP_PROXY', value: 'http://hostname:3128/' }],
},
],
},
});
// When
const result = await extractProxyEnvVarsFromWorkspacePod('test-pod', 'test-namespace');
// Then
expect(result).toEqual({
httpProxy: 'http://hostname:3128/',
httpsProxy: null,
});
});

it('should return object with only HTTPS_PROXY if Proxy environment variables present', async () => {
// Given
k8sGetMocked.mockReturnValueOnce({
metadata: {
name: 'test-pod',
namespace: 'test-namespace',
},
spec: {
containers: [
{
env: [{ name: 'HTTPS_PROXY', value: 'https://hostname:3128/' }],
},
],
},
});
// When
const result = await extractProxyEnvVarsFromWorkspacePod('test-pod', 'test-namespace');
// Then
expect(result).toEqual({
httpProxy: null,
httpsProxy: 'https://hostname:3128/',
});
});
});