Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove third party test dependencies #297

Closed
yinzara opened this issue Apr 15, 2021 · 10 comments
Closed

Remove third party test dependencies #297

yinzara opened this issue Apr 15, 2021 · 10 comments
Milestone

Comments

@yinzara
Copy link

yinzara commented Apr 15, 2021

The github.com/gin-gonic/gin dependency has a http response splitting vulnerability in versions less than 1.7.0:
gin-gonic/gin#2632

Upgrading the dependency to 1.7.0 or higher will fix this vulnerability (current version is 1.7.1)

@nhooyr
Copy link
Contributor

nhooyr commented Aug 6, 2021

Hi @yinzara we don't actually use gin anywhere other than in tests but I'll be sure to do this in the next release.

@nhooyr
Copy link
Contributor

nhooyr commented Aug 6, 2021

And apologies for the delayed response.

@nhooyr nhooyr changed the title HTTP Response Splitting Vulnerability - github.com/gin-gonic/gin Upgrade gin in the next release Aug 6, 2021
@nhooyr nhooyr added this to the v1.8.8 milestone Aug 6, 2021
@jmichalak-fluxninja
Copy link

@nhooyr Any update on this? Github alerts me with GHSA-h395-qcrw-5vmq

@nhooyr
Copy link
Contributor

nhooyr commented Mar 6, 2023

Soon, my plan is to just move the third party tests to a different module. See #318 (comment)

@Roman-Bober
Copy link

Any updates on removing gin dependency soon ?, Blackduck scanner fail on this with high security error.

@prochac
Copy link

prochac commented Aug 8, 2023

Soon, my plan is to just move the third party tests to a different module. See #318 (comment)

It seems that it shouldn't be necessary.
https://gophers.slack.com/archives/C9BMAAFFB/p1691163449401899

We plan to release SQL util and want to test it as many SQL drivers as possible. So I was worried about the dependencies from tests.

@prochac
Copy link

prochac commented Aug 8, 2023

For others, feel free to remove gin from your indirect dependencies.

emcfarlane/larking#78 (comment)

@nhooyr
Copy link
Contributor

nhooyr commented Aug 18, 2023

https://gophers.slack.com/archives/C9BMAAFFB/p1691163449401899

@prochac can you copy and paste the contents of the link? I'm not a member of the Gophers Slack.

@prochac
Copy link

prochac commented Aug 18, 2023

https://gophers.slack.com/archives/C9BMAAFFB/p1691163449401899

@prochac can you copy and paste the contents of the link? I'm not a member of the Gophers Slack.

Tomáš Procházka
14 days ago (<- relative to today's day: 2023-08-18, this was posted on 2023-08-04)
Hi, how do you deal with test dependencies?
What I strugle with is that if I test my module of SQL library then I want to check compatibility with many possible SQL drivers.
That means a lot of indirect dependencies for people just using the code, not test.
Example from wild, the websocket package recomended as replacement for golang.org/x/net/websocket
#318
I was thinking of separate integration test module, that would "hide" the heavy test dependencies.
Any other recomendation/practise? (edited)

Bryan C. Mills
11 days ago
This is one of the motivating use-cases for module graph pruning, added in Go 1.17.

Bryan C. Mills
11 days ago
Even though your test dependencies do appear in the module graph for your users, they should have relatively little impact — your users will only need to download the module contents (or even the go.mod files) for the dependencies of the packages they actually import.

Bryan C. Mills
11 days ago
So I suggest putting the integration tests in a separate package in the same module. Your users won't import that package, so they won't download its dependencies, but it will still be tested against the same dependency versions as the user-facing packages — with no version skew like you might otherwise have in a separate test-only module.

Tomáš Procházka
10 days ago
Thanks for your reply. I would accept that, but what about people? Somehow the less deps the better is considered by the Go comunity.
#297
Here it seems that they angry with CVE report of test-only dependency.
Could be said, that it's false positive?
Here are people changing library because of indirect dependency, regardless it's indirect/test-only 😕
emcfarlane/larking#75 (edited)

Tomáš Procházka
10 days ago
I simulated the problem, and you right. It shouldn't matter:
Only go.sum is bloated, but that shouldn't really matter
https://github.com/prochac/huge-test-deps
https://github.com/prochac/user-of-huge-test-deps (edited)

Jason Lui
9 days ago
oh, so if integration tests are in a separate package, it wouldn’t show up in this module’s go.mod?

Bryan C. Mills
9 days ago
More than that, even: if the integration tests are in a separate package, its test dependencies shouldn't even show up in the consumer's go.sum.

Jason Lui
9 days ago
oh it still shows up in this module’s go.mod, but not in its users’ go.mod, hmm

Bryan C. Mills
9 days ago
Right. Your go.mod file lists your own dependencies, including test dependencies, but module graph pruning limits the impact of your dependencies if they are not relevant to your consumers.

Jason Lui
9 days ago
there’s concerns from users though, like what
@prochac
said, if I see a module with a lot of dependency in go.mod I’d rather not use it :thinking_face:
question: if I separate those dependencies in a separate require block and comment them manually, would that grouping and comment be preserved by go tools?

Bryan C. Mills
9 days ago
In general yes, although the logic to preserve those blocks has been somewhat prone to bugs. 😅

Tomáš Procházka
8 days ago
@jason Lui
that's what I am planning to do too. Building an alibi for bloated go.mod 😄 (edited)

Bryan C. Mills
8 days ago
FWIW, you can always suggest that your users run go list -deps and/or go list -deps -test on the package(s) they're considering in order to report their actual dependencies.

Jason Lui
8 days ago
but it’s too much, people usually see a long go.mod and closes the page 😂

nhooyr added a commit that referenced this issue Oct 13, 2023
The library we're currently using for protobufs is deprecated. Doesn't belong in the library core
anyway.

Closes #311
Updates #297
nhooyr added a commit that referenced this issue Oct 13, 2023
The library we're currently using for protobufs is deprecated. Doesn't belong in the library core
anyway.

Closes #311
Updates #297
@nhooyr
Copy link
Contributor

nhooyr commented Oct 13, 2023

Done in dev.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants