Ubuntu
snapd package

inconsistent apps/key validation

Bug #1570914 reported by Jamie Strandboge
6
This bug affects 1 person
Affects Status Importance Assigned to Milestone
click-reviewers-tools (Ubuntu)
Fix Released
High
Jamie Strandboge
Xenial
Fix Released
High
Jamie Strandboge
snapd (Ubuntu)
Fix Released
High
Unassigned
Xenial
Fix Released
High
Unassigned
ubuntu-core-launcher (Ubuntu)
Fix Released
High
Jamie Strandboge
Xenial
Fix Released
High
Jamie Strandboge

Bug Description

The following snap.yaml:

name: network-manager
...
apps:
  NetworkManager:
    ...

results in the following:
Apr 15 14:09:02 localhost.localdomain ubuntu-core-launcher[1180]: appname snap.network-manager.NetworkManager not allowed

This is because verify_appname() in src/main.c does not allow upper case letters. This is easy enough to fix but then I noticed that:

1. docs/meta.md is silent on the issue of what is allowed for the keys in the apps dictionary (app names) of the snap.yaml
2. validate.go is not checking the contents of the key name in the apps dictionary

Looking at 15.04, I see this regex that applies to 'name': `^[A-Za-z0-9/. _#:-]*$` but this is too lenient for the app name for several reasons (so 15.04 was buggy). 16.04 uses this same regex for validating things in apps[key], but neglects to verify 'key' itself.

I suggest we:
1. update snapd to use "^[a-zA-Z](?:-?[a-zA-Z0-9])*$" (ie, the same as snap name but allow upper-case)
2. adjust docs/meta.md accordingly
3. adjust ubuntu-core-launcher accordingly

Jamie Strandboge (jdstrand)
Changed in snapd (Ubuntu):
status: New → Confirmed
Changed in ubuntu-core-launcher (Ubuntu):
status: New → Confirmed
Changed in snapd (Ubuntu):
importance: Undecided → High
Changed in ubuntu-core-launcher (Ubuntu):
importance: Undecided → High
Revision history for this message
Jamie Strandboge (jdstrand) wrote :

The review tools don't verify this name either. I'm adding a check now since it can be rolled out faster (and easily changed) since including '.' and '_' are delimiters in various parts of the system.

Jamie Strandboge (jdstrand)
Changed in click-reviewers-tools (Ubuntu):
status: New → Fix Committed
importance: Undecided → High
assignee: nobody → Jamie Strandboge (jdstrand)
Revision history for this message
Jamie Strandboge (jdstrand) wrote :

Uploaded review tools 0.41 and ubuntu-core-launcher 1.0.26 for this. If the regex is refined in snapd, I'll adjust.

Changed in ubuntu-core-launcher (Ubuntu):
assignee: nobody → Jamie Strandboge (jdstrand)
status: Confirmed → In Progress
status: In Progress → Fix Committed
Revision history for this message
Launchpad Janitor (janitor) wrote :

This bug was fixed in the package click-reviewers-tools - 0.41

---------------
click-reviewers-tools (0.41) xenial; urgency=medium

  * sr_lint.py: verify key name in the apps dictionary (LP: #1570914)

 -- Jamie Strandboge <email address hidden> Fri, 15 Apr 2016 10:24:17 -0500

Changed in click-reviewers-tools (Ubuntu):
status: Fix Committed → Fix Released
Revision history for this message
Launchpad Janitor (janitor) wrote :

This bug was fixed in the package ubuntu-core-launcher - 1.0.27

---------------
ubuntu-core-launcher (1.0.27) xenial; urgency=medium

  * src/main.c:
    - don't prepend snap. or snap_ since snapd is doing that for us now
      (LP: #1571048)
    - make whitelist_re strictly follow the 16.04 specification and adjust
      testsuite accordingly
  * debian/usr.bin.ubuntu-core-launcher: add locale and gconv reads for tr

ubuntu-core-launcher (1.0.26) xenial; urgency=medium

  * src/main.c: allow caps in appname (LP: #1570914)

 -- Jamie Strandboge <email address hidden> Fri, 15 Apr 2016 15:22:05 -0500

Changed in ubuntu-core-launcher (Ubuntu):
status: Fix Committed → Fix Released
Revision history for this message
Jamie Strandboge (jdstrand) wrote :

Assigning mvo for now (for SRU), but feel free to reassign.

Changed in snapd (Ubuntu):
assignee: nobody → Jamie Strandboge (jdstrand)
assignee: Jamie Strandboge (jdstrand) → nobody
assignee: nobody → Michael Vogt (mvo)
Michael Vogt (mvo)
Changed in snapd (Ubuntu Xenial):
assignee: Michael Vogt (mvo) → nobody
Revision history for this message
Michael Vogt (mvo) wrote :

We validate the app name in snapd now as well. This is available in xenial-updates.

Changed in snapd (Ubuntu):
status: Confirmed → Fix Released
Changed in snapd (Ubuntu Xenial):
status: Confirmed → Fix Released
To post a comment you must log in.
This report contains Public information  
Everyone can see this information.
Subscribing...

Other bug subscribers

Remote bug watches

Bug watches keep track of this bug in other bug trackers.