Mask API key in UI and update only on user edit

[tanushahande2003]

Added masking for API key in VRTExternalConfigurationsPage to prevent exposure. The real key is hidden with a generic mask, and only updated if the user changes the field. Existing key is preserved unless explicitly edited.

Description

Type of Change

• Bug fix - [ ] New feature - [ ] Breaking change - [ ] Plugin update

Checklist

• PR description clearly describes the changes
• Target branch is correct (master for features, Releases/* for fixes)
• Latest code from target branch merged
• No commented/junk code included
• No new build warnings or errors
• All existing unit tests pass
• New unit tests added for new functionality
• Cross-platform compatibility verified (Windows/Linux/macOS)
• CI/CD pipeline passes
• Code follows project conventions (Act{Platform}{Type}, {Platform}Driver)
• Repository objects use [IsSerializedForLocalRepository] where needed
• Error handling uses Reporter.ToLog() pattern
• Documentation updated for user-facing changes
• Self-review completed and code review comments addressed

Summary by CodeRabbit

• New Features

  • API keys are masked in the configuration UI: the real key is hidden and a fixed mask is shown while the actual value is preserved internally.

• Bug Fixes

  • Saving now detects when the masked field was edited, enforces a non-empty API key, and avoids unintentionally clearing the stored key.

[coderabbitai]

Warning

Rate limit exceeded

@tanushahande2003 has exceeded the limit for the number of commits that can be reviewed per hour. Please wait 9 minutes and 3 seconds before requesting another review.

⌛ How to resolve this issue?

After the wait time has elapsed, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

We recommend that you space out your commits to avoid hitting the rate limit.

🚦 How do rate limits work?

CodeRabbit enforces hourly rate limits for each developer per organization.

Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout.

Please see our FAQ for further information.

Walkthrough

Adds API key masking to the VRT External Configurations page: masks displayed API key, stores the mask in the TextBox.Tag, clears and restores the binding as needed, updates the underlying configuration only when the user replaces the masked value, and adds a using for System.Windows.Data.

Changes

Cohort / File(s)

Summary

API Key Masking Ginger/Ginger/ExternalConfigurations/VRTExternalConfigurationsPage.xaml.cs

Adds using System.Windows.Data. In SetControls applies validation for non-empty API key, clears the ApiKey TextBox binding, displays a fixed masked string and stores it in the TextBox.Tag. In xSaveButton_Click reads the Tag mask, detects if the user changed the visible text vs. the mask, validates and updates _VRTConfiguration.ApiKey only when changed, re-applies the mask/binding, then continues the existing save flow.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~12 minutes

Poem

🐰 I hid the key behind a mask so bright,
In a little Tag it naps by night,
If you hop and change it, I'll save with cheer,
If empty — I warn — come closer, dear,
A rabbit's tweak keeps secrets tight.

🚥 Pre-merge checks | ✅ 2 | ❌ 2

❌ Failed checks (1 warning, 1 inconclusive)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.
Description check	❓ Inconclusive	The description provides a brief summary of the changes but does not explicitly mark any checklist items as complete despite the changes being implemented.	Mark completed checklist items (e.g., PR description clarity, target branch correctness, no commented code) to indicate which validations have been satisfied.

✅ Passed checks (2 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly and concisely summarizes the main change: masking the API key in the UI and updating it only when the user edits it.
Merge Conflict Detection	✅ Passed	✅ No merge conflicts detected when merging into master

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing touches

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment
• Commit unit tests in branch issue_56007_Sensitive_Information_Disclosure

---

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.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

Actionable comments posted: 3

🤖 Fix all issues with AI agents

In `@Ginger/Ginger/ExternalConfigurations/VRTExternalConfigurationsPage.xaml.cs`:
- Around line 67-75: The current code clears the binding and sets a sentinel
mask on xAPIKeyTextBox.ValueTextBox when _VRTConfiguration.ApiKey is present;
replace this manual masking with a real PasswordBox bound via the existing
binding pattern (use BindingHandler.ObjFieldBinding with
PasswordBox.PasswordCharProperty) so the API key stays masked without removing
validation/bindings; instead of calling BindingOperations.ClearBinding or
setting the masked sentinel string/Tag, change the control to PasswordBox
(mirroring the pattern in CreateNewBranch.xaml.cs) and wire the same view-model
property (_VRTConfiguration.ApiKey) through the PasswordCharProperty binding so
validation and binding remain intact.
- Around line 101-114: In xSaveButton_Click, the current masked-text comparison
can still allow an empty string to overwrite _VRTConfiguration.ApiKey; update
the logic that checks apiKeyBox.Text vs masked to also verify the new value is
not null/empty before assigning to _VRTConfiguration.ApiKey and only then
restore apiKeyBox.Text to the masked value; if the new value is empty, do not
assign _VRTConfiguration.ApiKey and still re-mask the field (or leave it
unchanged) so clearing the masked field cannot save an empty API key (refer to
xAPIKeyTextBox, apiKeyBox.Tag/masked, and _VRTConfiguration.ApiKey).
- Around line 66-76: Clearing the binding with BindingOperations.ClearBinding on
xAPIKeyTextBox.ValueTextBox removes the ValidationRules added by
AddValidationRule (see ExtensionsMethods.AddValidationRule), so the "Key cannot
be empty" rule is lost; fix by either (A) attaching the validation independently
of the binding before calling ClearBinding (e.g., add the same ValidationRule
instance directly to the control's ValidationRules/validation collection or
re-register the rule on xAPIKeyTextBox.ValueTextBox after ClearBinding using the
same AddValidationRule helper), or (B) avoid removing the binding and instead
implement masking in the binding layer (use a converter on the binding to return
the masked string while preserving the original value/validation), referencing
_VRTConfiguration.ApiKey, BindingOperations.ClearBinding,
xAPIKeyTextBox.ValueTextBox, and the AddValidationRule method in
ExtensionsMethods.cs.

[coderabbitai]

Actionable comments posted: 4

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@Ginger/Ginger/ExternalConfigurations/VRTExternalConfigurationsPage.xaml.cs`:
- Around line 101-134: In xSaveButton_Click, when the empty API key is rejected
(the branch where string.IsNullOrWhiteSpace(apiKeyBox.Text) is true), add a
user-visible notification in addition to the Reporter.ToLog call: show a
MessageBox (or set a validation error on apiKeyBox) to inform the user that the
API key cannot be empty and that their input was not saved; keep the existing
mask/reset logic (BindingOperations.ClearBinding, apiKeyBox.Text = masked,
apiKeyBox.Tag = masked) and then return as before so behavior is unchanged
except for the visible feedback.
- Line 73: Extract the repeated mask literal into a single class-level constant
named ApiKeyMask and use it everywhere instead of the duplicated string;
specifically, add a private const string ApiKeyMask = "••••••••••••••••••••" to
the VRTExternalConfigurationsPage class and replace the local const string
masked and the other occurrences of the literal with ApiKeyMask so the equality
check that determines whether to overwrite the key uses the single shared
constant.
- Around line 114-115: The log level used when rejecting an empty API key should
be elevated from a warning to an error; update the call to Reporter.ToLog in the
VRTExternalConfigurationsPage code path that handles empty API keys (the
existing Reporter.ToLog(eLogLevel.WARN, "API Key cannot be empty") statement) to
use eLogLevel.ERROR so the condition is logged per the coding guidelines.
- Around line 67-76: The masked API key text is left editable and can be saved
back as the real key; add a focus handler on xAPIKeyTextBox.ValueTextBox (e.g.,
subscribe to GotFocus or PreviewMouseDown) that checks the Tag/Text marker used
for the mask (the same masked constant used when setting
xAPIKeyTextBox.ValueTextBox.Text and Tag), and if the marker is present clear
both Text and Tag so the user starts with an empty field before typing;
alternatively replace the editable TextBox with a PasswordBox (update XAML and
the code paths that read/set _VRTConfiguration.ApiKey) to avoid exposing or
accepting mask characters entirely.

---

Duplicate comments:
In `@Ginger/Ginger/ExternalConfigurations/VRTExternalConfigurationsPage.xaml.cs`:
- Around line 66-76: The binding clear removes the ValidateEmptyValue rule so
empty-key validation stops firing; fix by capturing the existing binding's
ValidationRules before calling BindingOperations.ClearBinding and then
re-attaching the validation rule(s) (at minimum the ValidateEmptyValue rule) to
the control after you clear the binding: call
BindingOperations.GetBinding(xAPIKeyTextBox.ValueTextBox, TextBox.TextProperty)
to retrieve the binding and its ValidationRules, perform
BindingOperations.ClearBinding(...), set the masked text/tag, then create or
reuse a Binding for TextBox.TextProperty and re-add the captured
ValidateEmptyValue rule (or reattach the original binding with the validation
rules) so the "Key cannot be empty" validation still runs.

[coderabbitai]

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@Ginger/Ginger/ExternalConfigurations/VRTExternalConfigurationsPage.xaml.cs`:
- Around line 109-113: The code currently allows saving an empty API key because
the if-block (checking masked vs apiKeyBox.Text) assigns
_VRTConfiguration.ApiKey = apiKeyBox.Text without guarding against an empty
string; add an explicit check before assignment in the same block that rejects
empty or whitespace apiKeyBox.Text (do not assign if
string.IsNullOrWhiteSpace(apiKeyBox.Text)), log the rejection using
Reporter.ToLog(eLogLevel.ERROR, "...") with a clear message, and show brief user
feedback (e.g., MessageBox) informing the user the empty key was not saved;
ensure you reference and protect the existing symbols apiKeyBox.Text and
_VRTConfiguration.ApiKey and perform this validation prior to the save that
follows later in the method.

---

Duplicate comments:
In `@Ginger/Ginger/ExternalConfigurations/VRTExternalConfigurationsPage.xaml.cs`:
- Around line 67-78: The API key textbox currently removes its binding and
writes a fixed masked string (masked constant and Tag) but has no GotFocus
handling, so users can click in and accidentally save mask bullets into the real
key; add a GotFocus handler on xAPIKeyTextBox.ValueTextBox that checks whether
Text (or Tag) equals the mask marker and if so clears Text and Tag and
re-establishes or re-applies the original binding/validation state (or sets a
flag to avoid persisting the mask), and ensure Save/Commit logic ignores the
mask value by checking against the mask constant before writing back to
_VRTConfiguration.ApiKey; reference xAPIKeyTextBox.ValueTextBox,
BindingOperations.ClearBinding, the masked constant and Tag when locating code
to change.
- Around line 67-78: The validation rule added to xAPIKeyTextBox.ValueTextBox is
lost by BindingOperations.ClearBinding when _VRTConfiguration.ApiKey is not
empty; to fix, preserve or re-attach validation after clearing the binding:
either capture existing ValidationRules (or re-create the ValidateEmptyValue
instance) and call xAPIKeyTextBox.ValueTextBox.AddValidationRule(...)
immediately after BindingOperations.ClearBinding, or replace the masking
approach by using a binding converter or switching xAPIKeyTextBox.ValueTextBox
to a PasswordBox so the original binding plus its validation rules are not
cleared; reference _VRTConfiguration.ApiKey, xAPIKeyTextBox.ValueTextBox,
BindingOperations.ClearBinding, and ValidateEmptyValue when making the change.