Codecov Report

Merging #2805 (49af546) into master (3efdd2c) will increase coverage by 0.00%.
The diff coverage is 100.00%.

@@           Coverage Diff           @@
##           master    #2805   +/-   ##
=======================================
  Coverage   98.06%   98.06%           
=======================================
  Files         132      132           
  Lines       11650    11655    +5     
=======================================
+ Hits        11424    11429    +5     
  Misses        154      154           
  Partials       72       72           

It's a bit fishy, not sure if it can be bypassed by encoding the dots. Using https://pkg.go.dev/net/url#URL.ResolveReference and seeing if the result is different than the input would be a more bulletproof way to do it. Also, it could be applied to the whole u := fmt.Sprintf("repos/%s/%s/contents/%s", owner, repo, escapedPath) path, to ensure there is no ../ in any of the other parameters either.

@molnarg - PRs are welcome. Thank you.

Could we get more information on the actual vulnerability this is protecting from, and the timeline to its resolution, if any ? Right now, I am considering multiple options:

• submitting a PR to use a catchable sentinel error
• submitting a PR to make that protection an opt-out
• waiting for the vulnerability to be fixed, then submitting a PR to remove that protection

As I cannot find any reference to the vulnerability in the GitHub API documentation, I have difficulty making an informed decision 🙂 .

@mrbobbytables - are you able to share any more information on the vulnerability that was reported to you?

@mrbobbytables : Would you happen to know (1) whether this vulnerability still exists and (2) whether the vulnerability affects any paths that literally contain .. as a substring (as this change implies), rather than just paths that contain .. as a path component (which is more understandable IMHO)? I ask because there's no good way to work around this check in cases where a file– or directory-name happens to actually include .., so if the check can safely be narrowed, that would be very helpful.