Publishing snap to edge channel changes store metadata

[Merlijn Sebrechts]

Publishing a snap to a branch on the edge channel overwrites the store metadata such as name, icon and description.

## Steps to reproduce

1. Publish a snap to the stable channel. The store metadata shows the metadata of that snap version.
2. Change the metadata and publish the snap to a branch in the edge channel. The store metadata now shows the metadata of the snap in the edge channel.

## What I expect to happen

* Publishing a snap to the stable channel changes the store metadata if it isn't manually set.
* Publishing a snap to a branch of the edge channel doesn't change the store metadata.

The user installs the stable channel by default so the user should see the metadata of the stable snap by default.

## Security issue

"branch" channels are documented as a good way to create snaps for test-versions. Our CI infrastructure is set up that an open PR will trigger a new push to a pr-specific branch in the edge channel. Thus, a malicious actor can change the snap metadata simply by creating a PR. For example, a malicious person could change the description to include a deprecation notice and point to a different installer that installs malware. From the user's point of view, this message will come from the verified publisher of the charm.

At the moment, our build infrastructure is set up so snaps will only get created for PR's of our core contributors. Thus, the security implication is small for us.

[Daniel Manrique (roadmr)]

What are you using to push your builds? i.e. you say "Every PR will trigger a new push to the edge/branch channel to make testing easier.". How is this done?

The reason I ask is this. The main issue here has already been reported (https://bugs.launchpad.net/snapstore/+bug/1782368) but we tried this extensively and the workaround is to use dashboard.snapcraft.io. The reason is that metadata (icon, description, summary...) that are edited in dashboard are marked specially, so when a push comes from snapcraft or Launchpad, dashboard notices a human has already modified the data and prevents the update by indicating there is a conflict.

So it would be good to know how you're publishing your snaps to see if there's something we missed.

[Daniel Manrique (roadmr)]

Matías has double-checked and modifying the metadata through dashboard and pushing a new revision afterwards won't override the metadata with the uploaded snapcraft.yaml, as I described. This makes this bug a duplicate of bug 1782368, and also makes it crucially important that you tell us how you're pushing your snaps so we can look at that specific scenario.

One additional comment - regardless of metadata behavior, it seems like a bad idea that "Every PR will trigger a new push to the edge/branch channel". What prevents anyone from not only modifying metadata but actually injecting malicious code into the snap? while snaps' contained nature helps mitigate here, it's still open to many of the same scenarios you mentioned with metadata. I could replace the snap's main command with one that prints a notice with the misleading text you mentioned and the users would be equally tricked into self-harm.

[Merlijn Sebrechts (merlijn-sebrechts)]

@Daniel

First of all, there is an issue that is not described in bug #1782368 and that issue is the cause of the security issue. Without manually set metadata, the store will show the metadata of _the latest published snap_ regardless of which channel and which branch that snap is pushed to.

The behaviour I expect here is that the store only shows the metadata of the latest _stable_ snap, not _any_ snap. By default, the user installs the latest stable snap, so by default, the user should see the metadata of the latest stable snap.

> Matías has double-checked and modifying the metadata through dashboard and pushing a new revision afterwards won't override the metadata with the uploaded snapcraft.yaml, as I described.

I just tested this and it seems to be correct. I was probably confused because the /accounts/snaps settings override the dashboard settings and remove the manual flag. I'll post in the other issue if this issue pops up again.

> it seems like a bad idea that "Every PR will trigger a new push to the edge/branch channel" What prevents anyone from not only modifying metadata but actually injecting malicious code into the snap?

Every PR will trigger a new push to a _branch_ in the edge channel. That branch is named according to the pr. For example: edge/yaru-pr674. This means that a user will never get the snap created by the attackers code, unless the user explicitly installs that branch. The branch won't show up on `snap info` nor in Ubuntu Software so there is no risk that a user would accidentally install that snap.

That said, didrocks just informed me that our automated build system only publishes the snaps of the PRs of our core contributors, so the security impact is only minimal for us.

[Daniel Manrique (roadmr)]

Thanks for the explanation!

Let's recap:

1- If metadata is set manually via "old" dashboard, automated uploads (snapcraft/Launchpad) will NOT clobber metadata set by a human. Actually, using "new" storefront at http://snapcraft.io/account/, since a fix was deployed for https://bugs.launchpad.net/bugs/1782368. Not a problem.

2- Your automated builds only publish PRs from core contributors, lessening the security impact for you. Not a problem (the discussion on whether this is a bad idea is orthogonal here).

3- The remaining thing here is that uploading a revision will attempt to update the *snap* metadata with what that revision has. This behavior is by design, since metadata (at least description, summary, icons, screenshots, license) is per *snap*, not per *revision*. In that sense I would characterize this as not a bug, since things are working as we designed and planned them.

I would suggest checking http://forum.snapcraft.io/ and searching for "metadata" so you can see the several discussions where API-updatable metadata was designed, discussed and implemented, and perhaps that would be a good place to present this case, so the group of people who helped design this feature can chime in. As is, there's no easy change to make here: I can't move metadata to be per revision (some of it makes sense, some doesn't), and we can't change things to update metadata for stable only because metadata is updated at *upload* time, not at *release* time. So a more careful consideration of this implication is warranted.

[Merlijn Sebrechts (merlijn-sebrechts)]

https://forum.snapcraft.io/t/mechanisms-for-converging-store-and-snap-metadata/2200/8?u=galgalesh

[Merlijn Sebrechts (merlijn-sebrechts)]

Note: this issue has been fixed in the latest update to the "update-metadata-on-release". The metadata now only gets updated when a release to `stable` happens.