Skip to content

Commit 038473c

Browse files
committed
Prevent secret values from leaking into the DOM when concealed
The conceal mechanism used a custom dot font to visually mask secret values, but the actual plaintext remained in the DOM. Selecting text (Ctrl+A or mouse selection) and copying would expose the real secret values. Replace the dot-font approach with a static placeholder that never renders real values in the DOM when concealed. Add unit tests for both KeyValue and DetailText components to verify secret values are not present in rendered HTML when concealment is active.
1 parent 690fb84 commit 038473c

4 files changed

Lines changed: 202 additions & 4 deletions

File tree

shell/components/DetailText.vue

Lines changed: 9 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+
>&#8226;&#8226;&#8226;&#8226;&#8226;&#8226;&#8226;&#8226;&#8226;&#8226;&#8226;&#8226;&#8226;&#8226;&#8226;&#8226;&#8226;&#8226;&#8226;&#8226;&#8226;&#8226;&#8226;&#8226;</span>
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>
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: 14 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -778,11 +778,18 @@ 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+
>
787+
&#8226;&#8226;&#8226;&#8226;&#8226;&#8226;&#8226;&#8226;&#8226;&#8226;&#8226;&#8226;&#8226;&#8226;&#8226;&#8226;&#8226;&#8226;&#8226;&#8226;
788+
</div>
781789
<TextAreaAutoGrow
782790
v-else-if="valueMultiline && row[valueName] !== undefined"
783791
v-model:value="row[valueName]"
784792
data-testid="value-multiline"
785-
:class="{'conceal': valueConcealed}"
786793
:disabled="disabled"
787794
:mode="mode"
788795
:placeholder="_valuePlaceholder"
@@ -941,6 +948,12 @@ export default {
941948
padding: 10px 10px 10px 10px;
942949
}
943950
951+
.concealed-value {
952+
padding: 10px;
953+
min-height: 40px;
954+
user-select: none;
955+
}
956+
944957
.text-monospace:not(.conceal) {
945958
font-family: monospace, monospace;
946959
}

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)