[BUG] Panel options persist even after deleting input option

I’ll explain with a simple example:

  1. In my custom panel setPanelOptions I have a radio using builder.addRadio({ path: "test" }).
  2. Then I create a dashboard using my custom panel.
  3. When I go to save my dashboard, I can see that my panel option value is saved in panels[n].options.test.
  4. Back in plugin development, I delete my radio button.
  5. I refresh my Grafana dashboard (or if this is a production build, I update the plugin manually.)
  6. I go in and edit some other panel options, and save the dashboard again.
  7. My old value from the deleted radio button is still there.

This issue can be fixed if you delete and re-add the custom panel, but this is definitely not a great solution. For example, if my plugin is published, many other people may be using it for visualization. I cannot expect everyone to delete and re-add the panel.

I can also see this being an issue for data validation. Let’s say I incorporate a text field that accepts an arbitrary string. I later change it to a dropdown list that does not accept custom input. The old corrupt string will still be saved in the panel options until the user goes in and selects a valid string from the dropdown, and saves the dashboard.

Hi @ventura this is not a bug but an expected behaviour. Your plugin is responsible of managing the values of its options using data migrations and fallbacks.

Let’s say I incorporate a text field that accepts an arbitrary string. I later change it to a dropdown list that does not accept custom input. The old corrupt string will still be saved in the panel options until the user goes in and selects a valid string from the dropdown, and saves the dashboard.

In this case, you introduced a breaking change and should handle it accordingly. Your plugin should implement logic to handle this case and show the correct option based on the old data (if any) or migrate the old data to your new supported structure.

Thanks for the clarification. This seems like a security risk to me.

One idea I have to mitigate this is to define a list of valid options and simply delete the “deleted” options. You could also validate input this way.

export const plugin = new PanelPlugin<MyOptions>(panel).setPanelOptions((builder, context) => {
  // Define a list of keys in your options interface
  const validOptions = ['optionA', 'optionB', 'optionC'];
  for (let option in context.options) {
    // Delete invalid options
    if (!validOptions.includes(option)) {
      delete context.options[option as keyof MyOptions];
    }
  }
  // Validate optionA
  const validValuesForOptionA = ['xyz', 'abc'];
  if (!validOptions.includes(context.options.optionA)) {
    // Set to a default value, or display a warning, etc.
  }
  return builder
    .addRadio({
      ...

If you don’t wish to maintain the validOptions array, I found this NPM package to generate an array of strings based on an interface: ts-transformer-keys

I’m still debating how I’m going to implement this. I still think it should be implemented directly in the Grafana plugin API. But since this is intended functionality and likely won’t change, hopefully this research may help others who come across this post in the future.

This seems like a security risk to me.

Could you elaborate on this?

One idea I have to mitigate this is to define a list of valid options and simply delete the “deleted” options.

this might work for some specific cases but not every plugin handles data migrations by removing invalid options. Some plugins might rename their options, or take the old value and convert it to something else.

If Grafana were to remove “old” options before they get the chance to migrate them we would introduce more complexity and restrictions to the platform and unexpected behaviour. That’s why is best to let plugins and developers handle their own data migrations.

For example, if a plugin requires an API key or some sort of token, and some of the panel options changed, it could “lock out” the user from manually deleting the token from the plugin options. They would need to delete the entire panel in order for that to happen.

Perhaps an option or API function to “force clean” panel data would be useful? Developers would need to explicitly call the function, but then would not need to worry about the cases I mentioned.

Regardless, I’m planning to implement this function in the next release of Psychart:

function cleanObject<T>(dirty: { [index: string]: any }, clean: T): T {
    for (let key in clean) {
        if (typeof clean[key] === typeof dirty[key]) {
            clean[key] = dirty[key];
        }
    }
    return clean;
}

It takes in a “dirty” object (the current panel options) and a known “clean” object (my preset default panel options - a object copy) and keeps current settings ONLY IF they match the type of the default setting. It’s not the most robust, (e.g. doesn’t validate typescript types, or look into deeper levels of the object) but it is so simple and maintainable that I plan to use it for every settings change.

That doesn’t pose a security risk; instead, it indicates a broken experience, which could be attributed to a bug in the plugin itself.

It is also very important to notice that panel options should never store any kind of secrets such as API keys or token. Panel options are stored in plain text. The security risk would be to store that kind of information in panel options.

The way to store API keys and tokens is by creating a datasource and using the secureJSONData to store it and fetch the data from the datasource.

Perhaps an option or API function to “force clean” panel data would be useful?

The correct way to do it is the plugin to handle its data migration, the cleanObject function you shared here is a good start. This is something plugins must implement themselves and it is not part of the plugin platform responsibility to keep the plugin’s data up to date with their own data schema.

Regardless, I’m planning to implement this function in the next release of Psychart

This can work in your specific case. Just keep in mind that in the future, you might want to rename a key and not just discard the old value.