Unable to access Secure Environment variable in Pull Request

Pure Krome's Avatar

Pure Krome

04 Mar, 2020 05:09 AM

Secure environment variables are unavailable in PR's (for security reasons) unless the UI has "Enable secure variables in Pull Requests from the same repository only" ticked.

I have this ticked but I still cannot access the secure env var.

Now my guess is this => it's not the same repository.

I fork upstream into my own repo (called origin) ... change code, submit a PR from origin -> upstream. Upstream CI kicks off .... and secure is not available.

So here's my problem:

When a PR arrives, I need to get a code coverage report. So I calculate the code coverage and wish to send the report up codecov.io so when I jump into the PR to review it, I can see a nice pretty chart with the current PR's code coverage stats. I can't push the codecov report up because the secure variables are not available.

Is this possible?

-PK-

  1. Support Staff 1 Posted by Feodor Fitsner on 04 Mar, 2020 06:32 PM

    Feodor Fitsner's Avatar

    Any PR from external repository is considered potentially insecure. Once the secret reaches build VM there is no way to prevent someone from revealing it if they can submit arbitrary code in the PR. Having full access to VM during the build there is no way to hide/encrypt the secret (or it's hard to implement reliably).

    How do you send those code coverage stats? Maybe we could implement a deployment provider that can be configured as environment?...

  2. 2 Posted by Pure Krome on 04 Mar, 2020 11:22 PM

    Pure Krome's Avatar

    Yep - I fully understand and agree re: insecurity. Hmm..

    How do you send those code coverage stats?

    Great question, here's a snippet of my appveyor.yml. I'm trying to use a MATRIX so I do a DEBUG and RELEASE build (at the same time). DEBUG builds do the code coverage. RELEASE builds does the test (with no code coverage).

    # We only:
    #   - Send DEBUG results up to codecov
    #   - Pack when it's RELEASE compiled assembly
    after_test:
      - ps: |
          if ($env:CONFIGURATION -eq 'Debug')
          {
              # The codecov bash script looks for the exact text 'true' for CI and APPVEYOR.
              # As such, we need to update this value when using a windows Image. Linux already sets this to 'true'.
              $env:APPVEYOR = 'true' 
              $env:CI = 'true'
    
              if ($env:codecov_token)
              {
                Write-Host "We have a code-cov token, so lets push the report up to codecov."
                Invoke-WebRequest -Uri 'https://codecov.io/bash' -OutFile codecov.sh
                bash codecov.sh -s './CodeCoverageResults/' -f '*.xml' -Z -t $env:codecov_token
              }
              else
              {
                  Write-Host "No codecov token provided. Probably because this a PR from a fork"
              }
          }
          
          dotnet pack src\<SNIP> -c $env:CONFIGURATION --include-symbols -p:SymbolPackageFormat=snupkg /p:Version=$env:APPVEYOR_BUILD_VERSION --no-build
    

    Ok. So here we can see that, for DEBUG, we:

    • download the codecov "UPLOAD" script.
    • push the results up to codecov. The script does a number of things, so it's not just a wrapper over some curl or httpclient, pushing a file up.

    notice this bit: => -t $env:codecov_token

    I originally had this:

    environment:
      codecov_token:
        secure: Tis+n7V0gFaHKdN.........
    

    but we know that's not going to work now.

    Questions:

    • does the upload have to occur during the after_test phase? No, but it feels right.
    • do we need to upload codecov results during PR's. I would say 'YES', especially for unknown people committing PR's. It really gives us a nice indication if their code has a risky impact because they haven't provided enough tests. (NOTE: coverage doesn't mean the code (that is covering) is good quality. Sure, they are testing their changes but the tests and/or changes are crap).

    So that's the workflow I'm looking at playing with.

    Lastly, I would have thought this is a solved problem already -> meaning, I'm not the first person to be doing this?

  3. Support Staff 3 Posted by Feodor Fitsner on 04 Mar, 2020 11:27 PM

    Feodor Fitsner's Avatar

    You are probably the first who is asking to do that in fork PRs...Anyway, codecov.sh looks non-trivial - there is no way we could re-implement that in C# :)
    That codecov token - does it have full rights to your CodeCov account or it has only "upload" permissions?

  4. Support Staff 4 Posted by Feodor Fitsner on 04 Mar, 2020 11:28 PM

    Feodor Fitsner's Avatar

    I was wrong about CodeCov client: https://github.com/codecov/codecov-exe

  5. Support Staff 5 Posted by Feodor Fitsner on 04 Mar, 2020 11:34 PM

    Feodor Fitsner's Avatar

    OK, let's imaging we add a new "CodeCov" deployment environment being able to upload CodeCov *.xml reports... Again, you'd need to allow deployments during PRs then which is also not good, right?

  6. 6 Posted by Pure Krome on 04 Mar, 2020 11:34 PM

    Pure Krome's Avatar

    So each github repo inside CodeCov has it's own token. It looks like it's an UPLOAD ONLY token.

    here's a screenie of one repo, for our GH org:

    Note the text there: Note: Token not required for public repositories uploading from Travis, CircleCI or AppVeyor.

    This is a PRIVATE repo.

  7. 7 Posted by Pure Krome on 04 Mar, 2020 11:36 PM

    Pure Krome's Avatar

    you'd need to allow deployments during PRs then which is also not good, right?

    Yep, agreed.

    One thought I had was: if this was a PRIVATE REPO, then does that mean all the people with access are "trusted"?

    If public, then we don't need a token (based on what that screen shot says).

  8. Support Staff 8 Posted by Feodor Fitsner on 04 Mar, 2020 11:41 PM

    Feodor Fitsner's Avatar

    Exactly! If it's a private repo then it's assumed the people are trusted as they already have access to the base repo. So, you just need to allow Enable secure variables in all Pull Requests then.

  9. 9 Posted by Pure Krome on 05 Mar, 2020 12:55 AM

    Pure Krome's Avatar

    Yep - i have already done that, though :( And it's not working because that check box is only for PR from the same repo.

    Forks are not the same repo.

    Hence my problem :(

    EDIT: Exact text from UI is: Enable secure variables in Pull Requests from the same repository only

  10. Support Staff 10 Posted by Feodor Fitsner on 05 Mar, 2020 12:57 AM

    Feodor Fitsner's Avatar

    What a minute... Enable secure variables in all Pull Requests is visible for private projects only. You said that was a private repo?

  11. 11 Posted by Pure Krome on 05 Mar, 2020 01:01 AM

    Pure Krome's Avatar

    /me checks.

  12. 12 Posted by Pure Krome on 05 Mar, 2020 01:03 AM

    Pure Krome's Avatar

    Oh FFS. it's public (which is ok) - i thought this was a private repo.

    Ok. so the tick box is useless for a public repo.

    codecov doesn't need a token for public repo's it seems.

    I'm going to test this out (no token AND the codecov script detects it's AppVeyor) and then confirm back.

    I've got a feeling that I might have just wasted your time and I'm starting to feel really bad, now.

    EDIT:

    (FWIW: we are a paying customer, though, with 2x parallel builds :) )

  13. 13 Posted by Pure Krome on 05 Mar, 2020 03:24 AM

    Pure Krome's Avatar

    Yep - confirmed. Eeks - all is good.

    It's a public repo, so codecov didn't need the upload token (which is sorta scary) and if this was a private repo, then that checkbox would work.

    Sorry Feodor to have potentially wasted some of your time.

  14. Pure Krome closed this discussion on 05 Mar, 2020 03:24 AM.

Comments are currently closed for this discussion. You can start a new one.

Keyboard shortcuts

Generic

? Show this help
ESC Blurs the current field

Comment Form

r Focus the comment reply box
^ + ↩ Submit the comment

You can use Command ⌘ instead of Control ^ on Mac

 

01 Oct, 2024 04:27 PM
26 Sep, 2024 03:49 PM
26 Sep, 2024 09:02 AM
25 Sep, 2024 07:07 PM
24 Sep, 2024 08:39 PM
24 Sep, 2024 06:47 AM