Skip to content

Commit c173594

Browse files
rak-philliprak-phillip
authored
Merge pull request #17024 from adifsgaid/fix-secret-values-exposed-on-select-all
Prevent secret values from leaking into the DOM when concealed
2 parents 7519fb9 + 5ac228f commit c173594

7 files changed

Lines changed: 211 additions & 24 deletions

File tree

‎pkg/rancher-components/src/components/Form/LabeledInput/LabeledInput.vue‎

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -395,7 +395,7 @@ export default defineComponent({
395395
:value="value || ''"
396396
:placeholder="_placeholder"
397397
autocapitalize="off"
398-
:class="{ conceal: type === 'multiline-password' }"
398+
:class="{ 'multiline-password': type === 'multiline-password' }"
399399
:aria-describedby="ariaDescribedBy"
400400
:aria-required="requiredField"
401401
@update:value="onInput"
@@ -464,6 +464,10 @@ export default defineComponent({
464464
</div>
465465
</template>
466466
<style scoped lang="scss">
467+
.multiline-password:not(:focus) {
468+
-webkit-text-security: disc;
469+
}
470+
467471
.labeled-input.view {
468472
input {
469473
text-overflow: ellipsis;

‎shell/assets/styles/app.scss‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,6 @@
1010
@import "./base/spacing";
1111

1212
@import "./fonts/fontstack";
13-
@import "./fonts/dots";
1413
@import "./fonts/zerowidthspace";
1514
@import "./fonts/icons";
1615

‎shell/assets/styles/fonts/_dots.scss‎

Lines changed: 0 additions & 18 deletions
This file was deleted.

‎shell/components/DetailText.vue‎

Lines changed: 12 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -187,10 +187,9 @@ export default {
187187
>{{ body }}</span>
188188

189189
<CodeMirror
190-
v-else-if="jsonStr"
190+
v-else-if="jsonStr && !concealed"
191191
:options="{mode:{name:'javascript', json:true}, lineNumbers:false, foldGutter:false, readOnly:true}"
192192
:value="jsonStr"
193-
:class="{'conceal': concealed}"
194193
aria-live="polite"
195194
/>
196195

@@ -199,9 +198,16 @@ export default {
199198
:class="{'conceal-wrapper': concealed}"
200199
>
201200
<span
201+
v-if="concealed"
202+
data-testid="detail-top_html"
203+
class="conceal"
204+
aria-live="polite"
205+
/>
206+
<span
207+
v-else
202208
v-clean-html="bodyHtml"
203209
data-testid="detail-top_html"
204-
:class="{'conceal': concealed, 'monospace': monospace && !isBinary}"
210+
:class="{'monospace': monospace && !isBinary}"
205211
aria-live="polite"
206212
/>
207213
</div>
@@ -270,6 +276,9 @@ export default {
270276
.conceal {
271277
white-space: nowrap;
272278
display: block;
279+
&::before {
280+
content: '••••••••••••••••••••••••';
281+
}
273282
}
274283
275284
.action-group {

‎shell/components/__tests__/DetailText.test.ts‎

Lines changed: 113 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,113 @@
1+
import { mount } from '@vue/test-utils';
2+
3+
import DetailText from '@shell/components/DetailText.vue';
4+
5+
jest.mock('@shell/utils/clipboard', () => ({ copyTextToClipboard: jest.fn() }));
6+
7+
describe('component: DetailText', () => {
8+
const defaultMocks = {
9+
$store: {
10+
getters: {
11+
'i18n/t': jest.fn((key: string) => `%${ key }%`),
12+
'prefs/get': jest.fn(() => true),
13+
}
14+
}
15+
};
16+
17+
describe('concealment', () => {
18+
it('should not render the actual secret value in the content area when concealed', () => {
19+
const secretValue = 'super-secret-password-xyz';
20+
const wrapper = mount(DetailText, {
21+
props: {
22+
value: secretValue,
23+
conceal: true,
24+
label: 'Password',
25+
},
26+
27+
global: {
28+
mocks: defaultMocks,
29+
directives: {
30+
'clean-html': () => {},
31+
'clean-tooltip': () => {},
32+
t: () => {},
33+
},
34+
stubs: {
35+
CopyToClipboard: true,
36+
CodeMirror: true,
37+
},
38+
},
39+
});
40+
41+
const concealedSpan = wrapper.find('[data-testid="detail-top_html"]');
42+
43+
expect(concealedSpan.exists()).toBe(true);
44+
expect(concealedSpan.classes()).toContain('conceal');
45+
expect(concealedSpan.text()).not.toContain(secretValue);
46+
});
47+
48+
it('should render the actual value when not concealed', () => {
49+
const visibleValue = 'visible-value-123';
50+
const wrapper = mount(DetailText, {
51+
props: {
52+
value: visibleValue,
53+
conceal: false,
54+
label: 'Data',
55+
},
56+
57+
global: {
58+
mocks: defaultMocks,
59+
directives: {
60+
'clean-html': (el: HTMLElement, binding: { value: string }) => {
61+
el.innerHTML = binding.value;
62+
},
63+
'clean-tooltip': () => {},
64+
t: () => {},
65+
},
66+
stubs: {
67+
CopyToClipboard: true,
68+
CodeMirror: true,
69+
},
70+
},
71+
});
72+
73+
const contentSpan = wrapper.find('[data-testid="detail-top_html"]');
74+
75+
expect(contentSpan.exists()).toBe(true);
76+
expect(contentSpan.classes()).not.toContain('conceal');
77+
});
78+
79+
it('should not render JSON secret values in CodeMirror when concealed', () => {
80+
const jsonSecret = '{"api_key": "secret-key-123"}';
81+
const wrapper = mount(DetailText, {
82+
props: {
83+
value: jsonSecret,
84+
conceal: true,
85+
label: 'Config',
86+
},
87+
88+
global: {
89+
mocks: defaultMocks,
90+
directives: {
91+
'clean-html': () => {},
92+
'clean-tooltip': () => {},
93+
t: () => {},
94+
},
95+
stubs: {
96+
CopyToClipboard: true,
97+
CodeMirror: true,
98+
},
99+
},
100+
});
101+
102+
const codeMirror = wrapper.findComponent({ name: 'CodeMirror' });
103+
104+
expect(codeMirror.exists()).toBe(false);
105+
106+
const concealedSpan = wrapper.find('[data-testid="detail-top_html"]');
107+
108+
expect(concealedSpan.exists()).toBe(true);
109+
expect(concealedSpan.classes()).toContain('conceal');
110+
expect(concealedSpan.text()).not.toContain('secret-key-123');
111+
});
112+
});
113+
});

‎shell/components/form/KeyValue.vue‎

Lines changed: 15 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -778,11 +778,16 @@ export default {
778778
@onInput="onInputMarkdownMultiline(i, $event)"
779779
@onFocus="onFocusMarkdownMultiline(i, $event)"
780780
/>
781+
<div
782+
v-else-if="valueConcealed"
783+
class="concealed-value conceal"
784+
data-testid="concealed-value"
785+
:aria-label="t('generic.ariaLabel.value', {index: i+1})"
786+
/>
781787
<TextAreaAutoGrow
782788
v-else-if="valueMultiline && row[valueName] !== undefined"
783789
v-model:value="row[valueName]"
784790
data-testid="value-multiline"
785-
:class="{'conceal': valueConcealed}"
786791
:disabled="disabled"
787792
:mode="mode"
788793
:placeholder="_valuePlaceholder"
@@ -941,6 +946,15 @@ export default {
941946
padding: 10px 10px 10px 10px;
942947
}
943948
949+
.concealed-value {
950+
padding: 10px;
951+
min-height: 40px;
952+
user-select: none;
953+
&::before {
954+
content: '••••••••••••••••••••';
955+
}
956+
}
957+
944958
.text-monospace:not(.conceal) {
945959
font-family: monospace, monospace;
946960
}

‎shell/components/form/__tests__/KeyValue.test.ts‎

Lines changed: 66 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -177,6 +177,72 @@ describe('component: KeyValue', () => {
177177
expect(firstValueInput.element.value).toBe('testvalue1');
178178
});
179179

180+
describe('valueConcealed', () => {
181+
it('should not render actual secret values in the DOM when valueConcealed is true', () => {
182+
const secretValue = 'super-secret-api-key-12345';
183+
const wrapper = mount(KeyValue, {
184+
props: {
185+
value: { mySecret: secretValue },
186+
mode: 'view',
187+
valueConcealed: true,
188+
},
189+
190+
global: {
191+
mocks: { $store: { getters: { 'i18n/t': jest.fn() } } },
192+
stubs: { CodeMirror: true },
193+
},
194+
});
195+
196+
const concealedEl = wrapper.find('[data-testid="concealed-value"]');
197+
198+
expect(concealedEl.exists()).toBe(true);
199+
expect(wrapper.html()).not.toContain(secretValue);
200+
});
201+
202+
it('should render a TextAreaAutoGrow with the real value when valueConcealed is false', () => {
203+
const secretValue = 'visible-value';
204+
const wrapper = mount(KeyValue, {
205+
props: {
206+
value: { myKey: secretValue },
207+
mode: 'view',
208+
valueConcealed: false,
209+
},
210+
211+
global: {
212+
mocks: { $store: { getters: { 'i18n/t': jest.fn() } } },
213+
stubs: { CodeMirror: true },
214+
},
215+
});
216+
217+
const concealedEl = wrapper.find('[data-testid="concealed-value"]');
218+
219+
expect(concealedEl.exists()).toBe(false);
220+
221+
const multilineEl = wrapper.find('[data-testid="value-multiline"]');
222+
223+
expect(multilineEl.exists()).toBe(true);
224+
});
225+
226+
it('should have user-select none on the concealed placeholder to prevent text selection', () => {
227+
const wrapper = mount(KeyValue, {
228+
props: {
229+
value: { mySecret: 'secret' },
230+
mode: 'view',
231+
valueConcealed: true,
232+
},
233+
234+
global: {
235+
mocks: { $store: { getters: { 'i18n/t': jest.fn() } } },
236+
stubs: { CodeMirror: true },
237+
},
238+
});
239+
240+
const concealedEl = wrapper.find('[data-testid="concealed-value"]');
241+
242+
expect(concealedEl.classes()).toContain('concealed-value');
243+
});
244+
});
245+
180246
it('a11y: adding ARIA props should correctly fill out the appropriate fields on the component', async() => {
181247
const value = [{ key: 'testkey1', value: 'testvalue1' }];
182248

0 commit comments

Comments
 (0)