Skip to content

Conversation

@ashb
Copy link
Member

@ashb ashb commented Mar 27, 2019

Reverts #4817

This is dangerous - it a user happens to run this on their host it will overwrite files!

cc @milton0825 @feng-tao

@milton0825
Copy link
Contributor

milton0825 commented Mar 27, 2019

@ashb I was assuming that we only invoke the script inside the Docker container but seems like that is not the case. Should we enforce that users to only run the script inside the container?

or we can use -i instead of -f

Cause ln to write a prompt to standard error if the target file exists.  If the
           response from the standard input begins with the character `y' or `Y', then
           unlink the target file so that the link may occur.  Otherwise, do not attempt
           the link.

@feng-tao
Copy link
Member

feng-tao commented Mar 28, 2019

@ashb will user run this script in local box instead of docker / CI? I don't get why user will like to run this script locally.

@ashb
Copy link
Member Author

ashb commented Mar 29, 2019

We only expect it to run in docker/docker-on-CI but if users do attempt to run this locally trying to follow run the CI locally we shouldn't overwrite files.

(I've had a report of this happening, so it's not an "it might happen" but "it has happened")

Given we also add to the end of authorized_keys maybe the better change here is to only run this script in the docker-container (by checking an env variable that we set somewhere?)

@ashb
Copy link
Member Author

ashb commented Mar 29, 2019

Closing in favour of #5003

@ashb ashb closed this Mar 29, 2019
@ashb ashb deleted the revert-4817-replace-existing-soft-link branch March 29, 2019 15:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants