From 0b0bf17a1b96c0be7d1399409f9f42e0af8ec211 Mon Sep 17 00:00:00 2001 From: Jon Jackson Date: Mon, 23 Feb 2026 15:47:13 -0500 Subject: [PATCH] OCPBUGS-70273: Prevent binary secret data corruption when editing When editing secrets containing binary data (JAR, JCEKS, etc), the binary values were being corrupted by decoding base64 as UTF-8 text, which inserts replacement characters for non-printable bytes. Fixed by: - Pass base64 data directly to DroppableFileInput for binary entries - Add isBase64Input prop to properly handle base64-encoded input - Preserve original binary values when form updates to prevent corruption - Use useMemo instead of useState for derived binaryData value - Use immutable reducer pattern to merge binary data Added Cypress test to verify editing text fields doesn't corrupt binary data in the same secret. Co-Authored-By: Claude Sonnet 4.5 --- .../formik-fields/DroppableFileInputField.tsx | 2 +- .../components/formik-fields/field-types.ts | 2 + .../tests/crud/secrets/key-value.cy.ts | 58 +++ .../create-secret/OpaqueSecretFormEntry.tsx | 3 +- .../create-secret/SecretFormWrapper.tsx | 30 +- .../public/components/utils/file-input.tsx | 446 ++++++++---------- frontend/public/locales/en/public.json | 6 +- 7 files changed, 293 insertions(+), 254 deletions(-) diff --git a/frontend/packages/console-shared/src/components/formik-fields/DroppableFileInputField.tsx b/frontend/packages/console-shared/src/components/formik-fields/DroppableFileInputField.tsx index 8df53e2f8fb..a84f1a551a3 100644 --- a/frontend/packages/console-shared/src/components/formik-fields/DroppableFileInputField.tsx +++ b/frontend/packages/console-shared/src/components/formik-fields/DroppableFileInputField.tsx @@ -24,7 +24,7 @@ const DroppableFileInputField: React.FC = ({ onChange && onChange(fileData); }} inputFileData={field.value} - inputFieldHelpText={helpText} + filenamePlaceholder={helpText} aria-describedby={helpText ? `${fieldId}-helper` : undefined} /> diff --git a/frontend/packages/console-shared/src/components/formik-fields/field-types.ts b/frontend/packages/console-shared/src/components/formik-fields/field-types.ts index a616c25111a..f60e5b53fe2 100644 --- a/frontend/packages/console-shared/src/components/formik-fields/field-types.ts +++ b/frontend/packages/console-shared/src/components/formik-fields/field-types.ts @@ -21,6 +21,8 @@ export interface FieldProps { export interface DroppableFileInputFieldProps extends FieldProps { onChange?: (fileData: string) => void; + helpText?: string; + label?: string; } export interface BaseInputFieldProps extends FieldProps { type?: TextInputTypes; diff --git a/frontend/packages/integration-tests-cypress/tests/crud/secrets/key-value.cy.ts b/frontend/packages/integration-tests-cypress/tests/crud/secrets/key-value.cy.ts index bcfd5a3cb4c..fea05df048e 100644 --- a/frontend/packages/integration-tests-cypress/tests/crud/secrets/key-value.cy.ts +++ b/frontend/packages/integration-tests-cypress/tests/crud/secrets/key-value.cy.ts @@ -132,4 +132,62 @@ describe('Create key/value secrets', () => { }); }); }); + + it('Validate editing text field does not corrupt binary data (OCPBUGS-70273)', () => { + const mixedSecretName = `key-value-mixed-secret-${testName}`; + const textKey = 'textfield'; + const textValue = 'original-password'; + const updatedTextValue = 'updated-password'; + const binaryKey = 'binaryfield'; + + // Create a secret with both text and binary data using CLI + cy.exec( + `oc create secret generic ${mixedSecretName} -n ${testName} --from-literal=${textKey}=${textValue} --from-file=${binaryKey}=${Cypress.config( + 'fileServerFolder', + )}/fixtures/${binaryFilename}`, + ); + + // Capture the original binary data + cy.exec( + `oc get secret -n ${testName} ${mixedSecretName} --template '{{.data.${binaryKey}}}'`, + ).then((originalBinary) => { + // Edit the secret via the console + cy.visit(`/k8s/ns/${testName}/secrets/${mixedSecretName}`); + detailsPage.isLoaded(); + detailsPage.clickPageActionFromDropdown('Edit Secret'); + + // Modify only the text field + cy.byTestID('secret-key') + .should('have.length', 2) + .each(($el) => { + if ($el.val() === textKey) { + // Find the corresponding value textarea and update it + cy.byLegacyTestID('file-input-textarea').first().clear().type(updatedTextValue); + } + }); + + // Verify binary field shows the binary alert (indicates it's still treated as binary) + cy.byTestID('file-input-binary-alert').should('exist'); + + secrets.save(); + cy.byTestID('loading-indicator').should('not.exist'); + detailsPage.isLoaded(); + + // Verify the text field was updated + secrets.clickRevealValues(); + cy.byTestID('copy-to-clipboard').should('contain.text', updatedTextValue); + + // Verify the binary data was NOT corrupted + cy.exec( + `oc get secret -n ${testName} ${mixedSecretName} --template '{{.data.${binaryKey}}}'`, + ).then((updatedBinary) => { + expect(updatedBinary.stdout).to.equal(originalBinary.stdout); + }); + + // Cleanup + cy.exec(`oc delete secret -n ${testName} ${mixedSecretName}`, { + failOnNonZeroExit: false, + }); + }); + }); }); diff --git a/frontend/public/components/secrets/create-secret/OpaqueSecretFormEntry.tsx b/frontend/public/components/secrets/create-secret/OpaqueSecretFormEntry.tsx index ab7598b4e94..8e7773401a8 100644 --- a/frontend/public/components/secrets/create-secret/OpaqueSecretFormEntry.tsx +++ b/frontend/public/components/secrets/create-secret/OpaqueSecretFormEntry.tsx @@ -62,7 +62,8 @@ export const OpaqueSecretFormEntry: FCC = ({ = (props) => { return acc; }, {}), ); + // Store binary data separately to preserve it during edits + const binaryData = useMemo( + () => + Object.entries(props.obj?.data ?? {}).reduce>((acc, [key, value]) => { + if (isBinary(null, Buffer.from(value, 'base64'))) { + acc[key] = value; + } + return acc; + }, {}), + [props.obj?.data], + ); const [base64StringData, setBase64StringData] = useState(props?.obj?.data ?? {}); const [disableForm, setDisableForm] = useState(false); const title = useSecretTitle(isCreate, formType); @@ -74,7 +85,20 @@ export const SecretFormWrapper: FCC = (props) => { const onDataChanged = (secretsData) => { setStringData({ ...secretsData?.stringData }); - setBase64StringData({ ...secretsData?.base64StringData }); + // Preserve binary values by merging them with form data + // Only backfill missing keys from binaryData, don't overwrite edited entries + const mergedData = Object.entries(binaryData).reduce( + (acc, [key, value]) => { + // Only add binary entry if it's missing from form data + if (acc[key] === undefined) { + acc[key] = value; + } + // Otherwise keep the existing value from form data + return acc; + }, + { ...secretsData?.base64StringData }, + ); + setBase64StringData(mergedData); }; const onError = (err) => { diff --git a/frontend/public/components/utils/file-input.tsx b/frontend/public/components/utils/file-input.tsx index 313d4f16e02..fcd21545085 100644 --- a/frontend/public/components/utils/file-input.tsx +++ b/frontend/public/components/utils/file-input.tsx @@ -1,264 +1,214 @@ -import { Component } from 'react'; -import { css } from '@patternfly/react-styles'; -import { NativeTypes } from 'react-dnd-html5-backend'; -import { DropTarget } from 'react-dnd'; -import { ConnectDropTarget, DropTargetMonitor } from 'react-dnd/lib/interfaces'; -import { Alert } from '@patternfly/react-core'; -/* eslint-disable-next-line */ -import { withTranslation, WithTranslation } from 'react-i18next'; +import type { FC, ReactNode } from 'react'; +import { useCallback, useState } from 'react'; +import { + Alert, + FileUpload, + FileUploadProps, + DropzoneErrorCode, + TextArea, + FormHelperText, + HelperText, + HelperTextItem, + FileUploadHelperText, + Spinner, + spinnerSize, + FormGroup, +} from '@patternfly/react-core'; import { isBinary } from 'istextorbinary'; +import { useTranslation } from 'react-i18next'; +import { units } from './units'; +import styles from '@patternfly/react-styles/css/components/FileUpload/file-upload'; -import withDragDropContext from './drag-drop-context'; +/** Maximal file size, in bytes, that user can upload */ +const MAX_UPLOAD_SIZE = 4000000; -// Maximal file size, in bytes, that user can upload -const maxFileUploadSize = 4000000; - -class FileInputWithTranslation extends Component { - constructor(props) { - super(props); - this.onDataChange = this.onDataChange.bind(this); - this.onFileUpload = this.onFileUpload.bind(this); - } - - onDataChange(event) { - this.props.onDataChange(event.target.value); - } - - onFileUpload(event) { - this.props.onFileChange(event.target.files[0]); - } - - render() { - const { - connectDropTarget, - errorMessage, - fileIsBinary, - hideContents, - isOver, - canDrop, - id, - isRequired, - t, - } = this.props; - const klass = css('co-file-dropzone-container', { - 'co-file-dropzone--drop-over': isOver, - }); - return connectDropTarget( -
- {canDrop && ( -
-

{t('public~Drop file here')}

-
- )} - -
- -
-
- - - - - - {t('public~Browse...')} - -
- {this.props.inputFieldHelpText ? ( -

- {this.props.inputFieldHelpText} -

- ) : null} - {!hideContents && ( - -