New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

`stack clean --full` deletes personal ssh keys when using docker #2000

Closed
tonyd256 opened this Issue Apr 5, 2016 · 23 comments

Comments

Projects
None yet
5 participants
@tonyd256

tonyd256 commented Apr 5, 2016

It appears that stack is linking one's home directory .ssh folder into the .stack-work/docker/_home/.ssh directory. This means if you remove your .stack-work folder or run stack clean --full it will try to delete your .ssh directory. It is failing to delete that directory because of this error: removeDirectoryRecursive: resource busy (Device or resource busy), but it is still deleting the contents which are the private and public keys. If you look at the output of the verbose command below you will see this part of the docker create command: -v /Users/tony/.ssh:/Users/tony/dev/thoughtbot/Doorbell/.stack-work/docker/_home/.ssh. This seems like the issue that will cause a stack clean --full to delete one's personal ssh keys. Could this instead mount the ssh directory to the linux equivalent to ~/.ssh?

Steps to reproduce:

  1. Run stack exec bash on a stack project using docker.
  2. Run ls .stack-work/docker/_home/.ssh.
  3. Observe your personal keys in that directory.
  4. Run exit to back out of the container.
  5. Run stack clean --full.
  6. Run ls -la ~/.ssh.
  7. Observe that your ssh keys are now gone.

Expected:

My ssh keys should still be there.

Actual:

My ssh keys have been deleted.

Here is the stack --version output:

$ stack --version
Version 1.0.4 x86_64

Here is the command I ran with --verbose:

$ stack clean --full --verbose
Version 1.0.4 x86_64
2016-04-05 11:38:58.675287: [debug] Checking for project config at: /Users/tony/dev/thoughtbot/Doorbell/stack.yaml @(stack_4dvKIEntzDN7ESZC7K4r0G:Stack.Config src/Stack/Config.hs:761:9)
2016-04-05 11:38:58.676911: [debug] Loading project config file stack.yaml @(stack_4dvKIEntzDN7ESZC7K4r0G:Stack.Config src/Stack/Config.hs:779:13)
2016-04-05 11:38:58.681428: [debug] Run process: docker --version @(stack_4dvKIEntzDN7ESZC7K4r0G:System.Process.Read src/System/Process/Read.hs:269:3)
2016-04-05 11:38:58.692452: [debug] Run process: docker inspect fpco/stack-build:lts-5.9 @(stack_4dvKIEntzDN7ESZC7K4r0G:System.Process.Read src/System/Process/Read.hs:269:3)
2016-04-05 11:38:58.750404: [debug] Run process: docker create --net=host -e STACK_IN_CONTAINER=1 -e STACK_ROOT=/Users/tony/.stack -e STACK_PLATFORM_VARIANT=dkda49f7ca9b244180d3cfb1987cbc9743 -e HOME=/Users/tony/dev/thoughtbot/Doorbell/.stack-work/docker/_home -e PATH=/opt/host/bin:/Users/tony/dev/thoughtbot/Doorbell/.stack-work/docker/_home/.local/bin:/opt/host/bin:/opt/stackage/lts-5/extra/bin:/opt/stackage/lts-5/ghc/bin:/opt/stackage/lts-5/tools/bin:/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin -e PWD=/Users/tony/dev/thoughtbot/Doorbell -v /Users/tony/.stack:/Users/tony/.stack -v /Users/tony/dev/thoughtbot/Doorbell:/Users/tony/dev/thoughtbot/Doorbell -v /Users/tony/dev/thoughtbot/Doorbell/.stack-work/docker/_home:/Users/tony/dev/thoughtbot/Doorbell/.stack-work/docker/_home -w /Users/tony/dev/thoughtbot/Doorbell -e USER=tony -v /Users/tony/.ssh:/Users/tony/dev/thoughtbot/Doorbell/.stack-work/docker/_home/.ssh -e SSH_AUTH_SOCK=/private/tmp/com.apple.launchd.sstCRpTmVo/Listeners -v /private/tmp/com.apple.launchd.sstCRpTmVo/Listeners:/private/tmp/com.apple.launchd.sstCRpTmVo/Listeners -v /Users/tony/.stack/programs/x86_64-linux/stack-1.0.4/stack:/opt/host/bin/stack -e PORT=8080 -t -i --env-file=.env fpco/stack-build:lts-5.9 /opt/host/bin/stack --internal-re-exec-version=1.0.4 --internal-docker-entrypoint "DockerEntrypoint {deUser = Nothing}" clean --full --verbose @(stack_4dvKIEntzDN7ESZC7K4r0G:System.Process.Read src/System/Process/Read.hs:269:3)
2016-04-05 11:38:58.806766: [debug] Run process: /usr/local/bin/docker start -a -i 26c280c718b7843a0ca59261d1fc8a1e76d9d7d1fa6c3936eb08a217be255a55 @(stack_4dvKIEntzDN7ESZC7K4r0G:System.Process.Run src/System/Process/Run.hs:105:5)
Version 1.0.4, Git revision cf18703b1392a96a5a4896a560309e501af63260 (3220 commits) x86_64
2016-04-05 18:38:59.855519: [debug] Checking for project config at: /Users/tony/dev/thoughtbot/Doorbell/stack.yaml @(stack_COhPD0SUWBs7s1VUFLBNcf:Stack.Config src/Stack/Config.hs:761:9)
2016-04-05 18:38:59.859384: [debug] Loading project config file stack.yaml @(stack_COhPD0SUWBs7s1VUFLBNcf:Stack.Config src/Stack/Config.hs:779:13)
2016-04-05 18:38:59.877821: [debug] Run process: ldd /opt/host/bin/stack @(stack_COhPD0SUWBs7s1VUFLBNcf:System.Process.Read src/System/Process/Read.hs:269:3)
2016-04-05 18:38:59.892539: [debug] Trying to decode /Users/tony/.stack/build-plan-cache/x86_64-linux-dkda49f7ca9b244180d3cfb1987cbc9743/lts-5.9.cache @(stack_COhPD0SUWBs7s1VUFLBNcf:Data.Binary.VersionTagged src/Data/Binary/VersionTagged.hs:55:5)
2016-04-05 18:38:59.920016: [debug] Success decoding /Users/tony/.stack/build-plan-cache/x86_64-linux-dkda49f7ca9b244180d3cfb1987cbc9743/lts-5.9.cache @(stack_COhPD0SUWBs7s1VUFLBNcf:Data.Binary.VersionTagged src/Data/Binary/VersionTagged.hs:64:13)
2016-04-05 18:38:59.921050: [debug] Trying to decode /Users/tony/.stack/indices/Hackage/00-index.cache @(stack_COhPD0SUWBs7s1VUFLBNcf:Data.Binary.VersionTagged src/Data/Binary/VersionTagged.hs:55:5)
2016-04-05 18:39:00.219557: [debug] Success decoding /Users/tony/.stack/indices/Hackage/00-index.cache @(stack_COhPD0SUWBs7s1VUFLBNcf:Data.Binary.VersionTagged src/Data/Binary/VersionTagged.hs:64:13)
2016-04-05 18:39:00.231919: [debug] Run process: ghc --info @(stack_COhPD0SUWBs7s1VUFLBNcf:System.Process.Read src/System/Process/Read.hs:269:3)
2016-04-05 18:39:00.279917: [debug] Run process: ghc --numeric-version @(stack_COhPD0SUWBs7s1VUFLBNcf:System.Process.Read src/System/Process/Read.hs:269:3)
2016-04-05 18:39:00.303065: [debug] Run process: ghc-pkg --no-user-package-db field --simple-output Cabal version @(stack_COhPD0SUWBs7s1VUFLBNcf:System.Process.Read src/System/Process/Read.hs:269:3)
2016-04-05 18:39:00.329028: [debug] Run process: ghc-pkg --no-user-package-db list --global @(stack_COhPD0SUWBs7s1VUFLBNcf:System.Process.Read src/System/Process/Read.hs:269:3)
/Users/tony/dev/thoughtbot/Doorbell/.stack-work/docker/_home/.ssh: removeDirectoryRecursive: resource busy (Device or resource busy)
2016-04-05 11:39:00.356765: [debug] Run process: docker rm -f 26c280c718b7843a0ca59261d1fc8a1e76d9d7d1fa6c3936eb08a217be255a55 @(stack_4dvKIEntzDN7ESZC7K4r0G:System.Process.Read src/System/Process/Read.hs:269:3)

@mgsloan mgsloan added the type: bug label Apr 6, 2016

@mgsloan mgsloan modified the milestones: pq, P1: Must Apr 6, 2016

@dysinger

This comment has been minimized.

Contributor

dysinger commented Apr 6, 2016

Ouch! Sorry about that.

@mgsloan

This comment has been minimized.

Collaborator

mgsloan commented Apr 6, 2016

Yeah, I think this is the worst stack bug to date. I'm impressed with the lack of expletives in this bug report ;)

@dysinger

This comment has been minimized.

Contributor

dysinger commented Apr 6, 2016

I have a patch

user@computer:~/src/github.com/commercialhaskell/stack% rm -rf .stack-work/docker .stack-docker
user@computer:~/src/github.com/commercialhaskell/stack% PAGER=cat git diff                     
diff --git a/stack.yaml b/stack.yaml
index 5f60225..fcac64a 100644
--- a/stack.yaml
+++ b/stack.yaml
@@ -1,4 +1,6 @@
 resolver: lts-5.0
+docker:
+  enable: true
 image:
   containers:
     - base: "fpco/ubuntu-with-libgmp:14.04"
user@computer:~/src/github.com/commercialhaskell/stack% stack exec -- bash -l
stack@computer:/home/user/src/github.com/commercialhaskell/stack$ 
stack@computer:/home/user/src/github.com/commercialhaskell/stack$ md5sum ~/.ssh/id_rsa
9e23e286185eb971d78e24e059d776f0  /home/user/src/github.com/commercialhaskell/stack/.stack-docker/_home/.ssh/id_rsa
stack@computer:/home/user/src/github.com/commercialhaskell/stack$ logout
user@computer:~/src/github.com/commercialhaskell/stack% md5sum ~/.ssh/id_rsa
9e23e286185eb971d78e24e059d776f0  /home/user/.ssh/id_rsa
user@computer:~/src/github.com/commercialhaskell/stack% stack clean --full
user@computer:~/src/github.com/commercialhaskell/stack% md5sum ~/.ssh/id_rsa
9e23e286185eb971d78e24e059d776f0  /home/user/.ssh/id_rsa
user@computer:~/src/github.com/commercialhaskell/stack% stack exec -- bash -l
stack@computer:/home/user/src/github.com/commercialhaskell/stack$ md5sum ~/.ssh/id_rsa
9e23e286185eb971d78e24e059d776f0  /home/user/src/github.com/commercialhaskell/stack/.stack-docker/_home/.ssh/id_rsa
stack@computer:/home/user/src/github.com/commercialhaskell/stack$ 
@dysinger

This comment has been minimized.

Contributor

dysinger commented Apr 6, 2016

I moved .stack-work/docker to .stack-docker. It no longer is included in the mass-delete that is stack clean --full

Additionally $HOME is now mounted in the container so we can just symlink it to the right place for the stack user.

dysinger added a commit that referenced this issue Apr 6, 2016

change the docker workdir
to $PROJECTROOT/.stack-docker

because `stack clean --full` tries to delete .stack-work/docker and it
can't because .stack-work/docker/_home is mounted by docker while it's
trying to delete it

resolves #2000
@silky

This comment has been minimized.

Contributor

silky commented Apr 6, 2016

so do you mean that if one were to delete .stack-docker it'd still potentially delete the keys? because i don't think that's particularly great ...

shouldn't all stack stuff stay in .stack-work?

@dysinger

This comment has been minimized.

Contributor

dysinger commented Apr 6, 2016

there's two problems here

  1. stack clean --full tries to delete everything under .stack-work (including mounted directories for the docker container) that simply wont work.
  2. the host user's actual .ssh dir was mounted in the path that gets recursively deleted also

I solved #1 by moving .stack-work/docker to .stack-docker out of the way so that's not deleted. stack clean --full doesn't try to delete dirs that are mounted now.

#1 solves #2 but we don't need to mount $HOME/.ssh explicitly if we just mount $HOME and symlink $HOME/.ssh to the stack user's $HOME.

@dysinger

This comment has been minimized.

Contributor

dysinger commented Apr 6, 2016

In any case we should never ever ever delete things from the host users .ssh dir. That's nasty.

@dysinger

This comment has been minimized.

Contributor

dysinger commented Apr 6, 2016

If we don't like moving .stack-work/docker to .stack-docker then we'll have to re-write stack clean so that it's not just a sledgehammer of destruction when we are using docker mount points.

@silky

This comment has been minimized.

Contributor

silky commented Apr 6, 2016

cool ok; i understand a bit better;

but am i right in understanding that if i were to look inside .stack-docker and delete things, that will be deleting things from my local filesystem that may have been mounted by stack?

@dysinger

This comment has been minimized.

Contributor

dysinger commented Apr 6, 2016

if you look at .stack-docker on the host, it'll have some misc files & symlinks in it. if you just recursively delete you'll delete a symlink (wrt .ssh). if you delete ~/.ssh inside the container it'll just delete the symlink. if you go crazy and delete everything out of ~/.ssh/ then yes, you just deleted your hosts .ssh content. so don't do that :)

@dysinger

This comment has been minimized.

Contributor

dysinger commented Apr 6, 2016

This is just a proposal at this point. I want more people to like or dislike the change/move to .stack-docker for docker container working files/mounts.

@dysinger

This comment has been minimized.

Contributor

dysinger commented Apr 6, 2016

thank you @tonyd256 for such an awesome bug report

@tonyd256

This comment has been minimized.

tonyd256 commented Apr 6, 2016

Thanks @dysinger and others! And thanks for all your great work on this amazing tool!

I'm curious why the .ssh directory is needed? or further, why there is a different $HOME specified? Would it be possible to leave these out of the docker command and let users add them in their stack.yaml file if they need them?

@dysinger

This comment has been minimized.

Contributor

dysinger commented Apr 6, 2016

@borsboom would know the answer to those two

@borsboom

This comment has been minimized.

Contributor

borsboom commented Apr 6, 2016

Sorry about this! It's the unintended consequence of a bad interaction between two requested features we implemented: that stack.yaml can reference private git repositories when using Docker, and that stack clean --full delete all the Stack state. A different $HOME is specified because chances are that dotfiles in your host's home directory won't work properly in the Docker container, especially if you're not running Ubuntu on the host (e.g. you can have stuff in your .bashrc that breaks the shell in the container).

@borsboom

This comment has been minimized.

Contributor

borsboom commented Apr 6, 2016

IMO the moving .stack-work/docker to .stack-docker should be a separate issue. The intent is that stack clean --full should delete the docker stuff too, so that will have to be solved a different way (probably by having it run outside the container).

I tried removing that commit from the PR, and the fix still works (~/.ssh remains intact after running stack clean --full). I also rebased against the stable branch while I was at it.

@borsboom borsboom assigned borsboom and unassigned dysinger Apr 6, 2016

@borsboom

This comment has been minimized.

Contributor

borsboom commented Apr 6, 2016

I've pushed my version to #2003

@mgsloan

This comment has been minimized.

Collaborator

mgsloan commented Apr 6, 2016

We should also put the fix on master!

@tonyd256

This comment has been minimized.

tonyd256 commented Apr 6, 2016

This is awesome! When will I be able to do stack upgrade to get this?

@mgsloan

This comment has been minimized.

Collaborator

mgsloan commented Apr 6, 2016

@tonyd256 Not yet, it needs to be uploaded to hackage first.

@borsboom

This comment has been minimized.

Contributor

borsboom commented Apr 7, 2016

I'm the process of building the binaries and running the integration tests on every supported platform. Once that's done, I'll upload to Hackage.

@borsboom

This comment has been minimized.

Contributor

borsboom commented Apr 7, 2016

v1.0.4.3 is released, and all official binaries and packages uploaded.

@tonyd256

This comment has been minimized.

tonyd256 commented Apr 7, 2016

Awesome! Thanks everyone!!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment