Skip to content

Disable feature toggle when featureMap attribute has external storage#1696

Open
ethanzhouyc wants to merge 7 commits into
project-chip:masterfrom
ethanzhouyc:external-feature
Open

Disable feature toggle when featureMap attribute has external storage#1696
ethanzhouyc wants to merge 7 commits into
project-chip:masterfrom
ethanzhouyc:external-feature

Conversation

@ethanzhouyc
Copy link
Copy Markdown
Collaborator

@ethanzhouyc ethanzhouyc commented Mar 10, 2026

When the featureMap attribute of a cluster has External storage, ZAP has no control over its value. Toggling the feature will cause mismatch between UI and database.

To solve this, this PR:

  • Disable feature toggle when featureMap attribute has external storage, and display warning to user
  • Added warning if user toggles a feature with external attribute depending on it
  • Added warning if user updates an attribute to external, and removes related warning if they switch back
  • Add unit tests

@gemini-code-assist
Copy link
Copy Markdown
Contributor

Summary of Changes

Hello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!

This pull request addresses an issue where ZAP's UI could get out of sync with the database when attempting to toggle features for clusters whose featureMap attribute is managed by an external storage mechanism. By detecting external storage for the featureMap attribute, the system now proactively disables the feature toggle in the UI and provides a clear warning to the user, ensuring data consistency and preventing erroneous operations.

Highlights

  • External Storage Handling: Implemented logic to disable feature toggling in the UI when a cluster's featureMap attribute is configured with external storage, as ZAP cannot control its value.
  • User Warning: Added a warning message to inform users when a feature toggle is disabled due to external featureMap storage, preventing UI/database mismatches.
  • Unit Tests: Included new unit tests to verify the correct behavior of feature toggling when the featureMap attribute has external or RAM storage options.

🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console.

Changelog
  • src-electron/rest/user-data.js
    • Added featureMapStorageOption to the request body in httpPostCheckConformOnFeatureUpdate to pass storage information to the conformance checker.
  • src-electron/validation/conformance-checker.js
    • Modified generateWarningMessage and checkElementConformance to accept featureMapStorageOption as a parameter.
    • Introduced a new check in generateWarningMessage to disable feature changes and generate a warning if the featureMap attribute's storage option is external.
  • src/util/feature-mixin.js
    • Updated API calls to include the featureMapAttribute?.storageOption when checking or confirming feature changes.
  • test/feature.test.js
    • Added new test cases to checkElementConformance to validate the behavior when the featureMap attribute has external storage, ensuring toggles are disabled and warnings are displayed.
    • Included a test case to confirm that toggling behaves normally when the featureMap attribute has RAM storage.
Activity
  • No specific activity (comments, reviews, progress updates) has been recorded for this pull request yet.
Using Gemini Code Assist

The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.

Invoking Gemini

You can request assistance from Gemini at any point by creating a comment using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands on the current page.

Feature Command Description
Code Review /gemini review Performs a code review for the current pull request in its current state.
Pull Request Summary /gemini summary Provides a summary of the current pull request in its current state.
Comment @gemini-code-assist Responds in comments when explicitly tagged, both in pull request comments and review comments.
Help /gemini help Displays a list of available commands.

Customization

To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.

Limitations & Feedback

Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for GitHub and other Google products, sign up here.

You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.

Footnotes

  1. Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution.

Copy link
Copy Markdown
Contributor

@gemini-code-assist gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request correctly implements the logic to disable feature toggling when the featureMap attribute has external storage, with well-contained changes and effective unit tests. However, it introduces a Cross-Site Scripting (XSS) vulnerability by concatenating untrusted user input into a warning message that is rendered as HTML in the frontend. This critical security issue must be addressed by sanitizing the input or disabling HTML rendering for these notifications. Additionally, there are minor suggestions to improve code style by using template literals for string construction, enhancing readability and maintainability.

Comment thread src-electron/validation/conformance-checker.js
Comment thread test/feature.test.js Outdated
@ethanzhouyc
Copy link
Copy Markdown
Collaborator Author

@brdandu pointed out that if user toggles a feature with external attributes conforming to it, in this case, ZAP cannot handle updating the value of the attributes, and we should throw a warning.

Addressed this issue in the latest ocmmits

Comment thread src-electron/db/query-session-notification.js
Comment thread src-electron/rest/user-data.js Outdated
Comment thread src-electron/validation/conformance-checker.js Outdated
// Check 0: if the featureMap attribute storage is external, ZAP cannot modify it
if (featureMapStorageOption === dbEnum.storageOption.external) {
result.warningMessage.push(
warningPrefix +
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you give a user use case on when this would show up in the notifications pane?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Toggling a feature with external attributes conforming to it, would trigger the warning.

Here is the real example I use:

  1. create endpoint on Extended Color Light
  2. go to color control cluster
  3. enable currentHue attribute, change storage from RAM to External, and then disable it
  4. toggle the HS feature

As currentHue has mandatory conformance on HS, this would trigger the warning:

⚠️ Check Feature Compliance on endpoint: 1, cluster: Color Control, attribute CurrentHue, required by feature: Hue And Saturation (HS), has external storage and ZAP does not have control over it.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What about existing applications which have this problem?

Comment thread src/util/feature-mixin.js
…om both attribute and feature toggle if switch away from External
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants