LSIF: Do not swallow errors on upload. #6648
Conversation
| return | ||
| } | ||
|
|
||
| if conf.Get().LsifEnforceAuth { | ||
| err, status := enforceAuth(w, r, repoName) | ||
| if err != nil { | ||
| http.Error(w, err.Error(), status) | ||
| return |
There was a problem hiding this comment.
Oof, that's scary, can you get SECURITY comments around this please? like this: https://sourcegraph.com/github.com/sourcegraph/sourcegraph@b6eb78abaa32584aed6641ee64bf27e76025c07c/-/blob/cmd/frontend/auth/providers/providers.go#L24-25
Does this mean 3.10 is vulnerable?
There was a problem hiding this comment.
I'll update. Cherry pick this fix?
There was a problem hiding this comment.
Also this doesn't leak any information, only allows us to accept uploads that we haven't verified the owner of. And it's only for dot-com, no one will enable this setting in enterprise.
There was a problem hiding this comment.
Does that still warrant a security guy, or is that reserved for private data warnings?
There was a problem hiding this comment.
It's auth-related, so please add a security comment here anyway.
If it only affects sourcegraph.com and enterprise customers wouldn't enable this, then fine to not cherry-pick.
There was a problem hiding this comment.
Updated. Could you check the comment?
Co-Authored-By: Stephen Gutekanst <stephen.gutekanst@gmail.com>
Co-Authored-By: Stephen Gutekanst <stephen.gutekanst@gmail.com>
We were treating all errors from GetRepo and ResolveRev as not found. This hides errors and does not give us anything to diagnose actual issues (input or backend).