Requesting auto-connection of personal-files to sam-cli


#1

Hi all, I’m working on snapping the AWS SAM CLI. You can see the recipe etc. in this pull request.

The SAM CLI needs personal-files because it relies on the AWS CLI which users will almost certainly have pre-installed and which stores its configuration in ~/.aws.


[revoked] Requesting classic confinement for `sam-cli`
#2

And while I’m doing a poke on the alias post for this, I might as well do this one too :slight_smile:


#3

The readonly access is clearly needed and I’m inclined to vote in favor of granting an installation constraint for read-only access to ~/.aws, but I’m somewhat hesitant because sam-cli is not the clear owner of this directory and this directory contains very sensitive information that would give the snap access to run up charges in AWS. I may feel less concern about this if the snap were named after the upstream project, aws-sam-cli. That said, the snap’s description is (currently) “The AWS SAM CLI tool for the AWS Serverless Application Model” so it isn’t trying to hide anything. Also, @stilvoid is in the process of upstreaming the snap packaging: https://github.com/awslabs/aws-sam-cli/commit/d48992ef6acdc2eceb739176bda92f1f3dd55d82

@stilvoid - Why did you choose to name the project sam-cli instead of aws-sam-cli?

As for auto-connection, I’m disinclined to grant auto-connection because the snap is not the clear owner. It is also easy for the snap to determine if it has read access, understand that it is a snap, and tell the user “Access denied to ~/.aws. Please run: sudo snap connect sam-cli:personal-files”. Personally, that would actually build confidence that the project cares about the security of this very sensitive data. Before I vote, I’d like to hear what an architect has to say (cc @pedronis).

Note that personal-files is new and we are still defining the processes around it. We’ve identified that personal-files is being used by more than just ‘owners of the directory’ as can be seen from sam-cli, Kubicorn and kubefwd.

(@stilvoid - feel free to ignore my comments from here down)

@pedronis, @mvo (who created the interface) - should snapd be updated so that a snap declaration can express the interface reference in its policy? Ie, let’s say we dictate as a matter of process for snaps like this that a snap use a particular interface reference, like so:

name: foo
summary: application for building upon bar
plugs:
  home-dot-bar:
    interface: personal-files
    read: [ $HOME/.foo ]

such that snap interfaces would list:

$ snap interfaces
...
:personal-files   foo:home-dot-bar

and users connect would snap connect foo:home-dot-bar. Currently, by design, there is no way to reference home-dot-bar in the snap declaration. Eg (written as yaml instead of json for easier reading):

personal-files
  allow-auto-connection :  true
  allow-installation: 
    plug-attributes:
      read: \\$HOME/\\.bar
      interface-reference: home-dot-bar  # doesn't exist

@pedronis, I could introduce this in the review-tools with an override mechanism in the short term, but this isn’t scalable (not to mention, getting the changes into prod in the store takes days) and I feel this needs to be enforced by snapd. At what priority should this be implemented? Who would pick up that work? (that can of course be discussed elsewhere).

@pedronis - specific to this request, how do you feel about the installation constraint? manual vs auto connect? How does that change if the above were implemented, if at all?


#4

We have plans to implement something like that, the spelling would be more like:

personal-files
  allow-auto-connection :  true
  allow-installation: 
    plug-names:
      - home-dot-bar
    plug-attributes:
      read: \\$HOME/\\.bar

About when it can happen, we need to see in a bit depending on progress on other things.


#5

if this get upstreamed (as seems being worked on), well it have the same publisher as aws-cli ?


#6

Hi folks, to answer a few points:

  • The review on GitHub has also expressed a preference to call the package aws-sam-cli and I completely agree. I’ll fix it later today.

  • For now, the package will be under my account but I am currently working with the owner of the aws publisher account to move a number of packages to that - this package will be part of that move :slight_smile:


#7

Nice. Would you recommend I add some ad-hoc override tests to the review-tools so that we can define our processes and enforce them at the store level in the meantime?


#8

All this is good news. I’d like to give this a little time to work itself out before casting my vote, because if it’s the same publisher and the name is changed to aws-sam-cli, I would vote in favor of auto-connect (new information notwithstanding).


#9

It’s worked itself out :wink:

aws-sam-cli is now registered under the AWS publisher and the snapcraft.yaml is merged and using the correct name: https://github.com/awslabs/aws-sam-cli/blob/develop/snap/snapcraft.yaml


#10

+1 for installation and auto-connection for read access to ~/.aws.

@reviewers - can others vote on this?


Interface autoconnection request for the glances snap
Gallery-dl: Previously granted *-files plugs now trigger manual review
#11

+1 from me. This makes sense given the publisher and the application.


#12

it’s not a bad idea if you can fit that in, given I don’t have a timeline for you for when we can implement that in snapd itself yet.


#13

FYI, I’ve implemented this in the review-tools. Please add to the snapd team’s roadmap since the check is limited and updating overrides.py in the review-tools won’t scale.


#14

2 votes for, 0 against. Granting installation and auto-connection for read access to ~/.aws to aws-sam-cli when the interface reference is ‘config-aws’. This is now live.

Note, you’ll need to update your snap to use:

plugs:
  config-aws:
    interface: personal-files
    read:
    - $HOME/.aws

In this manner, this can be manipulated via snap connect/disconnect with ‘aws:config-aws’