ACM-33048 Sensitive fields shown by default in the Review step#6011
Conversation
📝 WalkthroughWalkthroughAdds an optional per-input Changes
Sequence Diagram(s)sequenceDiagram
participant User as User
participant ReviewStep as ReviewStep
participant Registry as Registry
participant HoverZone as ReviewPenHoverZone
participant RevealControl as RevealButton
User->>ReviewStep: open review UI
ReviewStep->>Registry: fetch step input metadata (includes secret)
Registry-->>ReviewStep: return input nodes with secret flag
ReviewStep->>HoverZone: render row (masked if secret && no error)
HoverZone-->>RevealControl: render reveal control in controls cluster
User->>RevealControl: click toggle
RevealControl->>ReviewStep: update per-row showSecret state
ReviewStep-->>HoverZone: re-render row (masked <-> unmasked)
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~22 minutes 🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
frontend/packages/react-form-wizard/src/review/ReviewStep.tsx (1)
467-473:⚠️ Potential issue | 🟠 MajorMask secret values in collapsed review badges too.
When a section is collapsed, secret inputs still flow through
renderReviewInputDescriptionContent(child), which renders the raw value in the collapsed badge. That bypasses the new expanded-row masking path.Proposed fix
- const collapsedInputContent = child.error ? ( + const collapsedInputContent = child.error ? ( child.label ?? child.path + ) : child.secret ? ( + '****************' ) : child.value === true ? ( <CheckIcon aria-hidden /> ) : (🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/packages/react-form-wizard/src/review/ReviewStep.tsx` around lines 467 - 473, The collapsed badge path (collapsedInputContent) currently uses renderReviewInputDescriptionContent(child) which exposes raw values; change it to apply the same secret-masking logic used by the expanded row: detect secret inputs (e.g., child.secret or child.type === 'secret') inside the collapsed branch and render a masked placeholder (or call the same mask helper used by the expanded row) instead of renderReviewInputDescriptionContent, keeping the existing child.error and boolean-true CheckIcon branches intact; update collapsedInputContent to branch to the masked rendering when the input is secret.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@frontend/packages/react-form-wizard/src/review/ReviewStep.tsx`:
- Around line 716-726: The reveal toggle Button defined as revealButton lacks an
accessible name and state; update the Button (in ReviewStep.tsx where
revealButton is created) to include a dynamic aria-label (e.g., "Show secret"
when showSecrets is false and "Hide secret" when true) and expose its toggle
state via aria-pressed={showSecrets} (or aria-expanded if more appropriate for
your pattern), keeping the existing onClick handler and icon logic; ensure the
label text clearly reflects the action and updates when setShowSecrets toggles.
---
Outside diff comments:
In `@frontend/packages/react-form-wizard/src/review/ReviewStep.tsx`:
- Around line 467-473: The collapsed badge path (collapsedInputContent)
currently uses renderReviewInputDescriptionContent(child) which exposes raw
values; change it to apply the same secret-masking logic used by the expanded
row: detect secret inputs (e.g., child.secret or child.type === 'secret') inside
the collapsed branch and render a masked placeholder (or call the same mask
helper used by the expanded row) instead of renderReviewInputDescriptionContent,
keeping the existing child.error and boolean-true CheckIcon branches intact;
update collapsedInputContent to branch to the masked rendering when the input is
secret.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: c517861e-01f2-4dd0-b306-720e9480f313
📒 Files selected for processing (5)
frontend/packages/react-form-wizard/src/inputs/Input.tsfrontend/packages/react-form-wizard/src/review/ReviewStep.cssfrontend/packages/react-form-wizard/src/review/ReviewStep.tsxfrontend/packages/react-form-wizard/src/review/ReviewStepContexts.tsxfrontend/packages/react-form-wizard/src/review/ReviewStepNavigation.tsx
Signed-off-by: John Swanke <jswanke@redhat.com>
KevinFCormier
left a comment
There was a problem hiding this comment.
Looks good - just one little nit with types to fix up.
Signed-off-by: John Swanke <jswanke@redhat.com>
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
frontend/packages/react-form-wizard/src/review/ReviewStep.tsx (1)
467-473:⚠️ Potential issue | 🟠 MajorMask secrets in collapsed section badges.
Collapsed summaries still render
renderReviewInputDescriptionContent(child)for secret inputs, so a collapsed review section can show the raw token even though expanded rows are masked.Proposed fix
const collapsedInputContent = child.error ? ( child.label ?? child.path + ) : child.secret ? ( + '****************' ) : child.value === true ? ( <CheckIcon aria-hidden /> ) : ( renderReviewInputDescriptionContent(child)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/packages/react-form-wizard/src/review/ReviewStep.tsx` around lines 467 - 473, The collapsed summary currently uses renderReviewInputDescriptionContent(child) even for secret inputs; update the collapsedInputContent logic to detect secret/sensitive inputs (e.g., child.secret or child.type === 'secret') and replace the rendered raw value with a masked placeholder (like a fixed dot/asterisk string or a call to a new helper mask function) when the input is secret and not errored or a boolean true; ensure you reference the collapsedInputContent expression and renderReviewInputDescriptionContent(child) so the mask is applied only in collapsed view while keeping expanded masking behavior unchanged.frontend/packages/react-form-wizard/src/review/ReviewStepFindList.tsx (1)
160-167:⚠️ Potential issue | 🟠 MajorMask secret values in review search results too.
Search mode still builds
row.searchValuefrom the rawnode.value, then renders it in the find list. That exposes secret fields whenever review search is active and also indexes the raw secret in Fuse.Proposed fix
+const REVIEW_SECRET_MASK_TEXT = '****************' + function formatReviewFindSearchValue(node: WizardInputDomNode, labels: ReviewFindBooleanStrings): string { if (node.error) return node.error + if (node.secret && !isReviewValueUnset(node.value)) return REVIEW_SECRET_MASK_TEXT if (typeof node.value === 'boolean') { return node.value ? labels.reviewBooleanTrue : labels.reviewBooleanFalse }Also applies to: 417-447
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/packages/react-form-wizard/src/review/ReviewStepFindList.tsx` around lines 160 - 167, formatReviewFindSearchValue currently returns raw node.value (via formatReviewValueString) which leaks secrets into review search results and indexing; update this function to first check node.secret and, if true, return a masked placeholder instead of the raw value (use the existing masking helper if one exists or add a small maskSecretValue utility), and apply the same secret-checking/masking change wherever review search row.searchValue is constructed (notably in the other review formatting path that uses formatReviewValueString around lines referenced) so secrets are masked consistently when building searchValue and not emitted to Fuse.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In `@frontend/packages/react-form-wizard/src/review/ReviewStep.tsx`:
- Around line 467-473: The collapsed summary currently uses
renderReviewInputDescriptionContent(child) even for secret inputs; update the
collapsedInputContent logic to detect secret/sensitive inputs (e.g.,
child.secret or child.type === 'secret') and replace the rendered raw value with
a masked placeholder (like a fixed dot/asterisk string or a call to a new helper
mask function) when the input is secret and not errored or a boolean true;
ensure you reference the collapsedInputContent expression and
renderReviewInputDescriptionContent(child) so the mask is applied only in
collapsed view while keeping expanded masking behavior unchanged.
In `@frontend/packages/react-form-wizard/src/review/ReviewStepFindList.tsx`:
- Around line 160-167: formatReviewFindSearchValue currently returns raw
node.value (via formatReviewValueString) which leaks secrets into review search
results and indexing; update this function to first check node.secret and, if
true, return a masked placeholder instead of the raw value (use the existing
masking helper if one exists or add a small maskSecretValue utility), and apply
the same secret-checking/masking change wherever review search row.searchValue
is constructed (notably in the other review formatting path that uses
formatReviewValueString around lines referenced) so secrets are masked
consistently when building searchValue and not emitted to Fuse.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: de8e4034-6118-4d52-b950-775a7a3f45e5
📒 Files selected for processing (3)
frontend/packages/react-form-wizard/src/review/ReviewStep.tsxfrontend/packages/react-form-wizard/src/review/ReviewStepFindList.tsxfrontend/packages/react-form-wizard/src/review/ReviewStepNavigation.tsx
Signed-off-by: John Swanke <jswanke@redhat.com>
Signed-off-by: John Swanke <jswanke@redhat.com>
…-33048-Sensitive-fields-shown-by-default-in-the-Review-step Signed-off-by: John Swanke <jswanke@redhat.com> Made-with: Cursor # Conflicts: # frontend/packages/react-form-wizard/src/review/ReviewStep.tsx
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@frontend/packages/react-form-wizard/src/review/ReviewStep.tsx`:
- Around line 773-794: In useReviewSecretMaskState, the useState initializer
inverts masking when a row mounts unset; change the initial state to false (so
showSecrets defaults to false) instead of using isReviewValueUnset(value),
ensuring secrets are masked by default; keep the existing hasValue, masked,
maskedText and revealButton logic and only toggle showSecrets via setShowSecrets
when the user clicks the reveal button.
- Line 785: Replace the hardcoded English aria strings in ReviewStep.tsx with
entries from the StringContext: add two new keys (e.g. showSecretValue /
hideSecretValue) and an editLabel (or similar) to the WizardStrings type and
default strings in StringContext.tsx, then use useStringContext() in ReviewStep
to read those keys and replace aria-label={showSecrets ? 'Hide secret value' :
'Show secret value'} and the ariaLabel="Edit" props with the context values
(e.g. aria-label={strings.showSecretValue} / strings.hideSecretValue and
ariaLabel={strings.editLabel}) so all three strings are localized via
WizardStrings.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 2ff33a9f-71b0-4a12-a483-35ad54902e37
📒 Files selected for processing (7)
frontend/packages/react-form-wizard/src/inputs/Input.tsfrontend/packages/react-form-wizard/src/inputs/WizTextArea.tsxfrontend/packages/react-form-wizard/src/inputs/WizTextInput.tsxfrontend/packages/react-form-wizard/src/review/ReviewStep.cssfrontend/packages/react-form-wizard/src/review/ReviewStep.tsxfrontend/packages/react-form-wizard/src/review/ReviewStepContexts.tsxfrontend/packages/react-form-wizard/src/review/ReviewStepNavigation.tsx
💤 Files with no reviewable changes (2)
- frontend/packages/react-form-wizard/src/inputs/WizTextArea.tsx
- frontend/packages/react-form-wizard/src/inputs/WizTextInput.tsx
🚧 Files skipped from review as they are similar to previous changes (3)
- frontend/packages/react-form-wizard/src/review/ReviewStepContexts.tsx
- frontend/packages/react-form-wizard/src/inputs/Input.ts
- frontend/packages/react-form-wizard/src/review/ReviewStepNavigation.tsx
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: jeswanke, KevinFCormier The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
📝 Summary
Ticket Summary (Title):
ACM-33048 Sensitive fields shown by default in the Review step
Ticket Link:
https://redhat.atlassian.net/browse/ACM-33048
Type of Change:
✅ Checklist
General
ACM-12340 Fix bug with...)If Feature
If Bugfix
🗒️ Notes for Reviewers
Summary by CodeRabbit