apps:update sending testing settings via parameters seems like a dangerous footgun?

[joelhellman]

Expectations

Coming from ZAT and migrating Zendesk apps to ZCLI.

In ZAT, when I did zat create, I had to provide default values for settings, if app hadn't been installed before (I think only, if the field was set as required and didn't provide a default, but not to relevant to my feedback here).

in ZAT, when I did zat update for an installed, it updated the app, it didn't touch the app's installation settings. Make sense to me, since the user can change the installation settings once the app being used, so you want to be careful about overwriting the active installation setting when you just update your app, e.g. you fix a style issue or whatever.

I expected ZCLI do be closed to ZATs behavior when updating apps, but I got reality, which is that by default "testing settings" are sent to the customer's production app installation when running zat apps:update. Feels super dangerous to me.

Reality

In ZCLI, when I do an zcli apps:update, it now (since #14 was adressed)

1. includes and overwrite my app's installation settings with any settings set in "parameters" attribute in app.config.apps.json
2. even if I omit "parameters" property in app.config.apps.json or set it to null or {}, after app has been deployed, it prompts me for values. I can choose to ctrl-C or just press enter to not enter values, and if I don't it doesn't overwrite app installation settings (AFAIK)

The issue I have with 1) is:

• Undocumented - big gotcha when migrating apps from ZAT
• Dangerous default? Since zcli.apps.config.json now doubles as .zat (holds installation settings like app_id) and settings files (holds parameters), and that users with settings in their Zendesk apps would typically define parameters on zcli.apps.config.json for local testing, do the user expect these local testing settings to be copied and be applied on app updates?

The issue I have with 2) is:

• There is no way to tell zcli apps:update to ignore parameters set, which relates to 1. But it also seems to break CI, because if I just omit the parameter prop from zcli.apps.config.json (or set it till null or {}), I will get this interactive prompt for new values. Disregarding CI, this can also trip up other app developers who might think they should/have to provide a value in that prompt, when the app already have installation settings set by in Zendesk.

Steps to Reproduce

1. create a new Zendesk app using zcli apps:new
2. install app in Zendesk using zcli apps:create
3. an zcli.config.apps.json file should be created, content {"app_id": 123456}
4. add to app's manifest a warnBadLanguage parameter of type checkbox
5. change the zcli.config.apps.json to be {"app_id": 123456, "parameters": {"warnBadLanguage": true}}
6. update app using zcli apps:update
7. note that the app installation for the app now has setting.warnBadLanguage: true

In reality what then reproduces but seems like a dangerous operation

9. the dev does some local testing, so zcli.config.apps.json is changed to {"app_id": 123456, "parameters": {"warnBadLanguage": false}}
10. the dev fixes a style mistake and now runs zcli apps:update to push the update to the customer
11. the customer's app installation now has setting.warnBadLanguage: false

Issue details

• Command: zcli apps:server apps:update
• Version: @zendesk/zcli/1.0.0-beta.33 wsl-x64 node-v18.13.0
• OS: Unbuntu 22/WSL2

I run webpack, and so I have a /src/manifest.json
I build the distribution into /dist

So more or less what the default Zendesk zcli react scaffold does.

When I install the app the first time with zcli apps:create from my /dist, I get an output /dist/app.config.apps.json, and it has {"app_id": 123456}.

For a webpack config, I have to include the "parameters" with payload in app.config.apps.json in the /dist, or I cannot use the default values when doing local developing and testing. But I also have to include app.config.apps.json in the same /dist to run zcli apps:update, since app.config.apps.json also holds the app_id that maps to an installed instance.

With zat and webpack, a common setup is to copy the src/manifest.json into dist/manifest.json as we build (and watch) the output.

E.g:

    new CopyWebpackPlugin({
      patterns: [
        { from: "src/manifest.json", to: "../[name][ext]" },
        { from: "src/images/*", to: "./[name][ext]" },
      ],
    }),

Because zclip.apps.config.json now doubles as the old .zat (installation instance) and settings (local test settings), many developers might now attempt a quick fix like below, so testing settings gets into /dist:

    new CopyWebpackPlugin({
      patterns: [
        { from: "src/zcli.apps.config.json", to: "../[name][ext]" },
        { from: "src/manifest.json", to: "../[name][ext]" },
        { from: "src/images/*", to: "./[name][ext]" },
      ],
    }),

That is very dangerous, since it will then push the developers test settings on any update with zcli apps:update.

This is in fact also requested by an unhandled PR for the zcli react scaffold created at the start of this year: zendesk/app_scaffolds#5 which (lucklily?) didn't get merged yet.

As you see, its not webpack specific either - it will hit all developers that are testing apps with local test defaults.

The only workaround at the moment seems to for developer to wrap the zcli apps:update script (not the zat apps:server) so it removes the parameters property from the dist/zcli.apps.config.json, but keep it in src/zcli.apps.config.json (since they still want it for local dev). Since you do want the parameters in zcli.apps.config.json in /dist for local dev, the dev probably should not touch zcli apps:server and apps:update commands directly, but wrap those in npm run scripts or something.

There are at least 2 different ways of looking of how zat update should behave, I think.

The user who wants to update the app settings from a central deployment using zcli, and that have apps there the installation settings are never touched then changed by the Zendesk customer, or even by the developer with access to the Zendesk customer's instance, working in the Admin > Zendesk Support Apps interface. For this use case

• This use case was adressed by #14 I think.

Then we have users who have apps that have installation settings, that either the developer themselves or their customers are expected to change the app installation settings for once installed. This is probably the majority of Zendesk apps by the way.

• For these use cases, these apps have a high risk of (accidentally!) overwriting the Zendesk customers installation, once the dev push an update. And the users would probably not notice it right away, since the configuration pushed by the new zcli.apps.config.json, would only sometimes be different values, and since the values are using in testing, the values themselves would probably not throw any errors, so it would only be discovered by the Zendesk customer over time, as they see something (controlled by a app installation setting of their now updated app) is not behaving as expected.

Pretty dangerous behavior here!

Unless I've missed something, this current behavior since #14 seems really bad, and will throw surprises at everyone, I think.

If I was to request a bit on how we address this, we would reintroduce a separate settings file for ZCLI like ZAT had. This settings file wouldn't have to live in in /dist if the user didn't want to. Even if it did, it would only replace an app's installation settings with new values if some explicit flag was passed, e.g. to zcli apps:update.

Migrating from ZAT to ZCLI and hoping it makes developing Zendesk apps easier

For my situation, which I think it pretty common, a few different apps across different Zendesk accounts, needing to keep track of each apps install test settings and where they are installed so they can be updated when needed, and created when needed, I felt when was introduced some time ago, it was in scope for zcli to make this a bit easier. I don't feel zcli has achived this for me yet.

The current profiles:login command saves the current subdomain globally (in ~/.zcli). But I have separate apps with separate active domains per-app.

The profiles command doesn't help me switch settings, only do auth against a specific Zendesk account. For me the switching of Zendesk account is tied to switching of local test settings. If its just about switching auth to Zendesk, mapping auth with a subdomain as ~/.zat file made possible in ZAT, was easier for me.

I can imagine that your milage might vary here, and some users might have some common settings files so a straight up mapping become cumbersome if and handle lots of domains. I also get keyring is a bit safer, even if the ~/.zat doesn't leave my developer machine, but that's more arguable I think.

Another downside with zcli login, is you froze out or made things complicated for Windows users on WSL2 for quite a while (now Windows added support for graphical interface in WSL2) because your keyring implementation required a graphical interface on Ubuntu.

I think the documentation and especially the scaffolds should have some ideas how to handle settings and more than a single Zendesk app. Or the zcli command could have some good recipies to use to make this easier.

But right now, I feel both docs and zcli is not making it easy to build and deploy apps across multiple Zendesk accounts. I think it' quite common for Zendesk devs to have a least a sandbox, so workiong with at least two Zendesk accounts.

My old setup with ZAT using separate settings files

In my old setup for zat, I had a webpack setup for each app with with /src and /dist, and some different default settings file in a /profiles folder. Since I manage apps on a few domains I needed different profiles. I didn't want a global profile since I worked on different apps for different domains sometimes the same time.

My local setup per Zendesk app, was something like this:

profiles/settings.json                        <-- base settings
profiles/settings.mydomain1.json  <-- local test settings working on mydomain1, mydomain2, etc
profiles/settings.mydomain2.json       
profiles/settings.mydomain3.json       
profiles/.zat.mydomain1.json      <-- app_id etc for deployment on mydomain1, mydomain2, etc
profiles/.zat.mydomain2.json
profiles/.zat.mydomain3.json
.target/.settings                     <-- pointer to settings profile 
.target/.zat                              <-- pointer to .zat profile
  
dist/settings.json                 <-- currently active settings targeted by zat server (holds parameter settings)
dist/.zat                         <-- currently active deployment with app_id etc targeted by zat update (holds app_id)
~/.zat                            <-- mapping of subdomain and auth info

I then had a local script to load/switch between different environment. I'd do things like:

<myscript> load envOne
# work on envOne
# test envOne locally

<myscript> load envTwo
# test envTwo locally

<myscript> load envOne
zat update envOne

# maybe test in production
# feeling confident, update the rest

<myscript> load envTwo
zat update

<myscript> load envThree
zat update

I'd commit /profiles to repo. If an app was removed from an zendesk account, I'd remove the corresponding settings.mydomain and .zat.mydomain files from profiles.

Maybe something similar could be an idea to implement for the zcli tool?