Home → Suggestions →

Solution for leaking secure variables in PR's

• Edit

j.verdurmen

25 Nov, 2015 09:26 PM

> Secure variables are not decrypted during pull request (GitHub only)
builds of public projects. This is made intentionally to avoid leaking your
credentials by submitting PR with malicious build script displaying
environment variables.

This is really a big problem as I cannot validate in a PR if someone has broke the Xamarin build. This makes the use of Appveyor really less beneficial.

Right now there are 3 "solutions"

1) Ask people to PR to another branch, and after merge, merge to master
2) Get a second "master, master2 and merge from "master"
3) Don't use secure variables and just expose the username and password in the build script

All of them are horrible :(

But I think I have the solution:

Proposal:

1. Don't decrypt secure variables in PR's (like now)
2. Replace the %...% (or other syntax) as a string replace in the .appveyor script before execution. (so don't add them as environment variables)
3. Maybe protect the .appveyor script from being changed for a PR (or don't decrypt if .appveyor is changed)

Now I can run `appveyor RegisterXamarinLicense -Email %xamarin_email% -Password %xamarin_password%` in a PR, because RegisterXamarinLicense won't print or leak the secure variables.

1. Support Staff 1 Posted by Feodor Fitsner on 25 Nov, 2015 09:47 PM

   

   Now you can securely put your Xamarin credentials on project settings Xamarin tab and it will work for PRs too. Export to YML if you need to.

   - Feodor

   • Edit

2. 2 Posted by j.verdurmen on 25 Nov, 2015 10:01 PM

   

   Hi Feodor,

   Thanks for response, but I don't fully understand it.

   The secure variables aren't needed anymore? (Then http://www.appveyor.com/docs/lang/xamarin needs a small update :))

   • Edit

3. Support Staff 3 Posted by Feodor Fitsner on 25 Nov, 2015 10:04 PM

   

   Yes, secure variables are no longer needed (for Xamarin credentials) and yes, docs require to be updated.

   - Feodor

   • Edit

4. 4 Posted by j.verdurmen on 25 Nov, 2015 10:11 PM

   

   That's great!

   I tested it in short, is it possible to also secure the email address?

   Thanks for the quick responses!

   • Edit

5. Support Staff 5 Posted by Feodor Fitsner on 25 Nov, 2015 10:12 PM

   

   Yes, in appveyor.yml.

   - Feodor

   • Edit

6. 6 Posted by j.verdurmen on 25 Nov, 2015 10:15 PM

   

   Final question,

   I think this won't work in forks? How do I fix the build script that Xamarin builds are not run in the fork? (e.g. `if define`)

   Using CMD right now.

   • Edit

7. Support Staff 7 Posted by Feodor Fitsner on 25 Nov, 2015 10:18 PM

   

   Yes it won't work in forks if they are built under a different AppVeyor account. Configure credentials on project UI then.

   - Feodor

   • Edit

8. 8 Posted by j.verdurmen on 25 Nov, 2015 10:21 PM

   

   Yes, I did that, but I have still in my build script a command to build with Xamarin.

   That will crash in the fork I think, so I like to improve the build script that it will skip the Xamarin build in forks..

   • Edit

9. Support Staff 9 Posted by Feodor Fitsner on 25 Nov, 2015 10:29 PM

   

   Ah, I see what you mean. You can define some environment variable on main project's UI, say "main_repo" and then check it in build script. It won't be forked as it's not in appveyor.yml.

   - Feodor

   • Edit

10. 10 Posted by j.verdurmen on 25 Nov, 2015 10:42 PM

    

    That's a great idea!

    I created the environment variable 'build_with_xamarin' for that :)

    Thanks for your help! Great support!

    • Edit

11. Support Staff 11 Posted by Feodor Fitsner on 26 Nov, 2015 01:02 AM

    

    Great, I've also updated docs: http://www.appveyor.com/docs/lang/xamarin - thanks for the catch!

    • Edit

12. j.verdurmen closed this discussion on 03 Apr, 2017 08:57 PM.