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

SNOW-1856499 Separate GCS transport from the standard one #1274

Open
wants to merge 5 commits into
base: master
Choose a base branch
from

Conversation

sfc-gh-pfus
Copy link
Collaborator

@sfc-gh-pfus sfc-gh-pfus commented Dec 11, 2024

Description

Using the same HTTP pool caused problems on Windows.

@sfc-gh-pfus sfc-gh-pfus force-pushed the tmp-gcs-head-testing branch 4 times, most recently from 841f755 to c2d88a0 Compare December 12, 2024 12:59
Comment on lines 407 to 410
TLSClientConfig: &tls.Config{
RootCAs: certPool,
VerifyPeerCertificate: verifyPeerCertificateSerial,
},

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Semgrep identified an issue in your code:

MinVersion is missing from this TLS configuration. By default, TLS 1.2 is currently used as the minimum when acting as a client, and TLS 1.0 when acting as a server. General purpose web applications should default to TLS 1.3 with all other protocols disabled. Only where it is known that a web server must support legacy clients with unsupported an insecure browsers (such as Internet Explorer 10), it may be necessary to enable TLS 1.0 to provide support. Add `MinVersion: tls.VersionTLS13' to the TLS configuration to bump the minimum version to TLS 1.3.

To resolve this comment:

No guidance has been designated for this issue. Fix according to your organization's approved methods.

💬 Ignore this finding

Leave a nosemgrep comment directly above or at the end of line 407 like so // nosemgrep: go.lang.security.audit.crypto.missing-ssl-minversion.missing-ssl-minversion

Take care to validate that this is not a true positive finding before ignoring it.
Learn more about ignoring code, files and folders here.

You can view more details about this finding in the Semgrep AppSec Platform.

@sfc-gh-pfus sfc-gh-pfus changed the title Test GCS HEAD requests with retry NO-SNOW Separate GCS transport from the standard one Dec 13, 2024
Copy link

codecov bot commented Dec 13, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 82.19%. Comparing base (f8baf23) to head (b74c2e2).

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #1274      +/-   ##
==========================================
- Coverage   82.20%   82.19%   -0.02%     
==========================================
  Files          55       55              
  Lines       13484    13484              
==========================================
- Hits        11085    11083       -2     
- Misses       2399     2401       +2     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@sfc-gh-pfus sfc-gh-pfus marked this pull request as ready for review December 13, 2024 10:53
@sfc-gh-pfus sfc-gh-pfus requested a review from a team as a code owner December 13, 2024 10:53
@sfc-gh-pfus sfc-gh-pfus changed the title NO-SNOW Separate GCS transport from the standard one SNOW-1856499 Separate GCS transport from the standard one Dec 13, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants