Skip to content

Ensure lint workflow checks whether 3rd party license and code is up to date - #11047

Merged
andyfeller merged 8 commits into
trunkfrom
andyfeller/9422-license-compliance
Jun 23, 2025
Merged

Ensure lint workflow checks whether 3rd party license and code is up to date#11047
andyfeller merged 8 commits into
trunkfrom
andyfeller/9422-license-compliance

Conversation

@andyfeller

Copy link
Copy Markdown
Contributor

fixes #9422
fixes github/cli#111

This pull request addresses GitHub CLI obligation to comply with the licenses of our 3rd party dependencies.

These changes update the lint workflow to verify license information is up to date including source code certain licenses require redistributing source code around.

Additionally, this pull request introduces scripts/license and scripts/license-check to help maintainers and contributors regenerate this information easily.

This commit introduces the use of `go-licenses` within CI/CD and manual processes for generating / updating the license information used by GitHub CLI including the code required by license to be redistributed.

During GitHub CLI pull requests, the `lint` workflow will notify users if this information is not updated.
Copilot AI review requested due to automatic review settings May 30, 2025 16:54
@andyfeller
andyfeller requested a review from a team as a code owner May 30, 2025 16:54
@andyfeller
andyfeller requested a review from babakks May 30, 2025 16:54

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull Request Overview

This PR adds automated checks and tooling to ensure third-party license information is kept up to date, including source files for licenses that require redistribution.

  • Introduce script/licenses to generate license reports and copy necessary license files
  • Add script/licenses-check and update .github/workflows/lint.yml to fail CI if license docs aren’t current
  • Add documentation (docs/license-compliance.md) and a Go-licenses template for consistent output

Reviewed Changes

Copilot reviewed 1028 out of 1028 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
.github/workflows/lint.yml Include license compliance paths and add a “Check licenses” step
script/licenses New script to generate third-party license files
script/licenses-check New script to verify license documentation is up to date
docs/license-compliance.md Documentation for maintaining and checking license compliance
.github/licenses.tmpl Go-licenses template for generating markdown listings
Comments suppressed due to low confidence (2)

script/licenses-check:1

  • Ensure this script file has execute permissions (e.g., chmod +x script/licenses-check) so that CI and local invocations can run it directly.
#!/bin/bash

script/licenses:1

  • Ensure this script file has execute permissions (e.g., chmod +x script/licenses) so it can be executed in CI and by contributors.
#!/bin/bash

Comment thread third-party-licenses.windows.md
logger.Infof(
"logDNSError ID mismatch chosenServer=[%s] hostname=[%s] respHostname=[%s] queryType=[%s] msg=[%s] resp=[%s] err=[%s]",
chosenServer,
hostname,

Check failure

Code scanning / CodeQL

Log entries created from user input

This log entry depends on a [user-provided value](1). This log entry depends on a [user-provided value](2). This log entry depends on a [user-provided value](3).
// Otherwise log a general DNS error
logger.Infof("logDNSError chosenServer=[%s] hostname=[%s] queryType=[%s] err=[%s]",
chosenServer,
hostname,

Check failure

Code scanning / CodeQL

Log entries created from user input

This log entry depends on a [user-provided value](1). This log entry depends on a [user-provided value](2). This log entry depends on a [user-provided value](3).

Copilot Autofix

AI about 1 year ago

To fix the issue, the hostname variable should be sanitized before being logged. This involves removing newline characters (\n, \r) and other potentially harmful characters from the user input. The strings.ReplaceAll function can be used for this purpose. The sanitization should be applied in the logDNSError function in third-party/github.com/letsencrypt/boulder/bdns/dns.go, where the hostname is logged.

Suggested changeset 1
third-party/github.com/letsencrypt/boulder/bdns/dns.go

Autofix patch

Autofix patch
Run the following command in your local git repository to apply this patch
cat << 'EOF' | git apply
diff --git a/third-party/github.com/letsencrypt/boulder/bdns/dns.go b/third-party/github.com/letsencrypt/boulder/bdns/dns.go
--- a/third-party/github.com/letsencrypt/boulder/bdns/dns.go
+++ b/third-party/github.com/letsencrypt/boulder/bdns/dns.go
@@ -678,5 +678,8 @@
 		// Otherwise log a general DNS error
+		// Sanitize hostname to prevent log injection
+		sanitizedHostname := strings.ReplaceAll(hostname, "\n", "")
+		sanitizedHostname = strings.ReplaceAll(sanitizedHostname, "\r", "")
 		logger.Infof("logDNSError chosenServer=[%s] hostname=[%s] queryType=[%s] err=[%s]",
 			chosenServer,
-			hostname,
+			sanitizedHostname,
 			queryType,
EOF
@@ -678,5 +678,8 @@
// Otherwise log a general DNS error
// Sanitize hostname to prevent log injection
sanitizedHostname := strings.ReplaceAll(hostname, "\n", "")
sanitizedHostname = strings.ReplaceAll(sanitizedHostname, "\r", "")
logger.Infof("logDNSError chosenServer=[%s] hostname=[%s] queryType=[%s] err=[%s]",
chosenServer,
hostname,
sanitizedHostname,
queryType,
Copilot is powered by AI and may make mistakes. Always verify output.
if r.code == 0 {
r.code = http.StatusOK
}
return r.ResponseWriter.Write(body)

Check warning

Code scanning / CodeQL

Reflected cross-site scripting

Cross-site scripting vulnerability due to [user-provided value](1). Cross-site scripting vulnerability due to [user-provided value](2).

Copilot Autofix

AI about 1 year ago

To fix the issue, we need to ensure that any user-controlled data written to the HTTP response is properly sanitized or escaped. In this case, we can use the html.EscapeString function from the html package to escape special HTML characters in the body parameter before writing it to the response. This will prevent malicious scripts from being executed in the browser.

The fix involves:

  1. Importing the html package in measured_http/http.go.
  2. Escaping the body parameter using html.EscapeString before writing it to the response in the Write method of responseWriterWithStatus.

Suggested changeset 1
third-party/github.com/letsencrypt/boulder/metrics/measured_http/http.go

Autofix patch

Autofix patch
Run the following command in your local git repository to apply this patch
cat << 'EOF' | git apply
diff --git a/third-party/github.com/letsencrypt/boulder/metrics/measured_http/http.go b/third-party/github.com/letsencrypt/boulder/metrics/measured_http/http.go
--- a/third-party/github.com/letsencrypt/boulder/metrics/measured_http/http.go
+++ b/third-party/github.com/letsencrypt/boulder/metrics/measured_http/http.go
@@ -5,2 +5,3 @@
 	"strconv"
+	"html"
 
@@ -30,3 +31,4 @@
 	}
-	return r.ResponseWriter.Write(body)
+	escapedBody := []byte(html.EscapeString(string(body)))
+	return r.ResponseWriter.Write(escapedBody)
 }
EOF
@@ -5,2 +5,3 @@
"strconv"
"html"

@@ -30,3 +31,4 @@
}
return r.ResponseWriter.Write(body)
escapedBody := []byte(html.EscapeString(string(body)))
return r.ResponseWriter.Write(escapedBody)
}
Copilot is powered by AI and may make mistakes. Always verify output.
config := &tls.Config{
// Set InsecureSkipVerify to skip the default validation we are
// replacing. This will not disable VerifyConnection.
InsecureSkipVerify: true,

Check failure

Code scanning / CodeQL

Disabled TLS certificate check

InsecureSkipVerify should not be used in production code.

Copilot Autofix

AI about 1 year ago

To address the issue, we should avoid setting InsecureSkipVerify: true and instead use the VerifyConnection function for custom validation while keeping the default certificate verification enabled. This can be achieved by removing the InsecureSkipVerify field from the tls.Config struct. The VerifyConnection function will still be executed, allowing for custom validation.


Suggested changeset 1
third-party/github.com/letsencrypt/boulder/observer/probers/tls/tls.go

Autofix patch

Autofix patch
Run the following command in your local git repository to apply this patch
cat << 'EOF' | git apply
diff --git a/third-party/github.com/letsencrypt/boulder/observer/probers/tls/tls.go b/third-party/github.com/letsencrypt/boulder/observer/probers/tls/tls.go
--- a/third-party/github.com/letsencrypt/boulder/observer/probers/tls/tls.go
+++ b/third-party/github.com/letsencrypt/boulder/observer/probers/tls/tls.go
@@ -112,5 +112,4 @@
 	config := &tls.Config{
-		// Set InsecureSkipVerify to skip the default validation we are
-		// replacing. This will not disable VerifyConnection.
-		InsecureSkipVerify: true,
+		// Use VerifyConnection for custom validation while keeping default
+		// certificate verification enabled.
 		VerifyConnection: func(cs tls.ConnectionState) error {
EOF
@@ -112,5 +112,4 @@
config := &tls.Config{
// Set InsecureSkipVerify to skip the default validation we are
// replacing. This will not disable VerifyConnection.
InsecureSkipVerify: true,
// Use VerifyConnection for custom validation while keeping default
// certificate verification enabled.
VerifyConnection: func(cs tls.ConnectionState) error {
Copilot is powered by AI and may make mistakes. Always verify output.
case "GET":
base64Request, err := url.QueryUnescape(request.URL.Path)
if err != nil {
rs.log.Debugf("Error decoding URL: %s", request.URL.Path)

Check failure

Code scanning / CodeQL

Log entries created from user input

This log entry depends on a [user-provided value](1).

Copilot Autofix

AI about 1 year ago

To fix the issue, the user-provided value (request.URL.Path) should be sanitized before being logged. Specifically:

  1. Remove any newline characters (\n and \r) from the input to prevent log forgery in plain text logs.
  2. If the logs are displayed in HTML, encode the input using html.EscapeString to prevent HTML injection.

The best approach is to use strings.ReplaceAll to remove newline characters from the input before logging. This ensures compatibility with plain text logs and avoids introducing unnecessary dependencies.


Suggested changeset 1
third-party/github.com/letsencrypt/boulder/ocsp/responder/responder.go

Autofix patch

Autofix patch
Run the following command in your local git repository to apply this patch
cat << 'EOF' | git apply
diff --git a/third-party/github.com/letsencrypt/boulder/ocsp/responder/responder.go b/third-party/github.com/letsencrypt/boulder/ocsp/responder/responder.go
--- a/third-party/github.com/letsencrypt/boulder/ocsp/responder/responder.go
+++ b/third-party/github.com/letsencrypt/boulder/ocsp/responder/responder.go
@@ -36,2 +36,3 @@
 	"crypto"
+	"strings"
 	"crypto/sha256"
@@ -227,3 +228,5 @@
 		if err != nil {
-			rs.log.Debugf("Error decoding URL: %s", request.URL.Path)
+			sanitizedPath := strings.ReplaceAll(request.URL.Path, "\n", "")
+			sanitizedPath = strings.ReplaceAll(sanitizedPath, "\r", "")
+			rs.log.Debugf("Error decoding URL: %s", sanitizedPath)
 			rs.responseTypes.With(prometheus.Labels{"type": responseTypeToString[ocsp.Malformed]}).Inc()
EOF
@@ -36,2 +36,3 @@
"crypto"
"strings"
"crypto/sha256"
@@ -227,3 +228,5 @@
if err != nil {
rs.log.Debugf("Error decoding URL: %s", request.URL.Path)
sanitizedPath := strings.ReplaceAll(request.URL.Path, "\n", "")
sanitizedPath = strings.ReplaceAll(sanitizedPath, "\r", "")
rs.log.Debugf("Error decoding URL: %s", sanitizedPath)
rs.responseTypes.With(prometheus.Labels{"type": responseTypeToString[ocsp.Malformed]}).Inc()
Copilot is powered by AI and may make mistakes. Always verify output.
DialContext: df,
// We are talking to a client that does not yet have a certificate,
// so we accept a temporary, invalid one.
TLSClientConfig: &tls.Config{InsecureSkipVerify: true},

Check failure

Code scanning / CodeQL

Disabled TLS certificate check

InsecureSkipVerify should not be used in production code.

Copilot Autofix

AI about 1 year ago

To fix the issue, we should replace the use of InsecureSkipVerify: true with a more secure configuration. Specifically, we can use a custom tls.Config that validates certificates against a trusted certificate pool. If the application requires accepting self-signed certificates, we can explicitly load and trust those certificates instead of disabling verification entirely. This ensures that the application remains secure while still accommodating the specific requirements of the HTTP-01 validation process.

The changes will involve:

  1. Creating a custom tls.Config with a trusted certificate pool.
  2. Replacing the InsecureSkipVerify: true configuration with this custom configuration.

Suggested changeset 1
third-party/github.com/letsencrypt/boulder/va/http.go

Autofix patch

Autofix patch
Run the following command in your local git repository to apply this patch
cat << 'EOF' | git apply
diff --git a/third-party/github.com/letsencrypt/boulder/va/http.go b/third-party/github.com/letsencrypt/boulder/va/http.go
--- a/third-party/github.com/letsencrypt/boulder/va/http.go
+++ b/third-party/github.com/letsencrypt/boulder/va/http.go
@@ -5,2 +5,3 @@
 	"crypto/tls"
+	"crypto/x509"
 	"errors"
@@ -143,7 +144,10 @@
 func httpTransport(df dialerFunc) *http.Transport {
+	// Create a custom TLS configuration with a trusted certificate pool.
+	certPool := x509.NewCertPool()
+	// Add trusted certificates to the pool as needed.
+	// For example: certPool.AppendCertsFromPEM([]byte("..."))
 	return &http.Transport{
 		DialContext: df,
-		// We are talking to a client that does not yet have a certificate,
-		// so we accept a temporary, invalid one.
-		TLSClientConfig: &tls.Config{InsecureSkipVerify: true},
+		// Use the custom certificate pool for validation.
+		TLSClientConfig: &tls.Config{RootCAs: certPool},
 		// We don't expect to make multiple requests to a client, so close
EOF
@@ -5,2 +5,3 @@
"crypto/tls"
"crypto/x509"
"errors"
@@ -143,7 +144,10 @@
func httpTransport(df dialerFunc) *http.Transport {
// Create a custom TLS configuration with a trusted certificate pool.
certPool := x509.NewCertPool()
// Add trusted certificates to the pool as needed.
// For example: certPool.AppendCertsFromPEM([]byte("..."))
return &http.Transport{
DialContext: df,
// We are talking to a client that does not yet have a certificate,
// so we accept a temporary, invalid one.
TLSClientConfig: &tls.Config{InsecureSkipVerify: true},
// Use the custom certificate pool for validation.
TLSClientConfig: &tls.Config{RootCAs: certPool},
// We don't expect to make multiple requests to a client, so close
Copilot is powered by AI and may make mistakes. Always verify output.
records := []core.ValidationRecord{baseRecord}
numRedirects := 0
processRedirect := func(req *http.Request, via []*http.Request) error {
va.log.Debugf("processing a HTTP redirect from the server to %q", req.URL.String())

Check failure

Code scanning / CodeQL

Log entries created from user input

This log entry depends on a [user-provided value](1). This log entry depends on a [user-provided value](2).

Copilot Autofix

AI about 1 year ago

To fix the issue, the user-provided value (req.URL.String()) should be sanitized before being logged. Specifically:

  1. Remove any newline (\n) or carriage return (\r) characters from the string to prevent log forgery.
  2. Use strings.ReplaceAll to replace these characters with an empty string.
  3. Update the log statement to use the sanitized value.

The changes will be made to the log statement on line 489 in the file third-party/github.com/letsencrypt/boulder/va/http.go.


Suggested changeset 1
third-party/github.com/letsencrypt/boulder/va/http.go

Autofix patch

Autofix patch
Run the following command in your local git repository to apply this patch
cat << 'EOF' | git apply
diff --git a/third-party/github.com/letsencrypt/boulder/va/http.go b/third-party/github.com/letsencrypt/boulder/va/http.go
--- a/third-party/github.com/letsencrypt/boulder/va/http.go
+++ b/third-party/github.com/letsencrypt/boulder/va/http.go
@@ -488,3 +488,5 @@
 	processRedirect := func(req *http.Request, via []*http.Request) error {
-		va.log.Debugf("processing a HTTP redirect from the server to %q", req.URL.String())
+		sanitizedURL := strings.ReplaceAll(req.URL.String(), "\n", "")
+		sanitizedURL = strings.ReplaceAll(sanitizedURL, "\r", "")
+		va.log.Debugf("processing a HTTP redirect from the server to %q", sanitizedURL)
 		// Only process up to maxRedirect redirects
EOF
@@ -488,3 +488,5 @@
processRedirect := func(req *http.Request, via []*http.Request) error {
va.log.Debugf("processing a HTTP redirect from the server to %q", req.URL.String())
sanitizedURL := strings.ReplaceAll(req.URL.String(), "\n", "")
sanitizedURL = strings.ReplaceAll(sanitizedURL, "\r", "")
va.log.Debugf("processing a HTTP redirect from the server to %q", sanitizedURL)
// Only process up to maxRedirect redirects
Copilot is powered by AI and may make mistakes. Always verify output.
return err
}

va.log.Debugf("following redirect to host %q url %q", req.Host, req.URL.String())

Check failure

Code scanning / CodeQL

Log entries created from user input

This log entry depends on a [user-provided value](1). This log entry depends on a [user-provided value](2).

Copilot Autofix

AI about 1 year ago

To fix the issue, we need to sanitize the user-provided value (req.URL.String()) before logging it. Since the logs are likely plain text, we should remove any newline characters (\n and \r) from the string to prevent log forgery. This can be achieved using the strings.ReplaceAll function.

The fix involves:

  1. Sanitizing req.URL.String() by replacing newline characters with an empty string.
  2. Updating the log statement on line 571 to use the sanitized value.

Suggested changeset 1
third-party/github.com/letsencrypt/boulder/va/http.go

Autofix patch

Autofix patch
Run the following command in your local git repository to apply this patch
cat << 'EOF' | git apply
diff --git a/third-party/github.com/letsencrypt/boulder/va/http.go b/third-party/github.com/letsencrypt/boulder/va/http.go
--- a/third-party/github.com/letsencrypt/boulder/va/http.go
+++ b/third-party/github.com/letsencrypt/boulder/va/http.go
@@ -570,3 +570,5 @@
 
-		va.log.Debugf("following redirect to host %q url %q", req.Host, req.URL.String())
+		sanitizedURL := strings.ReplaceAll(req.URL.String(), "\n", "")
+		sanitizedURL = strings.ReplaceAll(sanitizedURL, "\r", "")
+		va.log.Debugf("following redirect to host %q url %q", req.Host, sanitizedURL)
 		// Replace the transport's DialContext with the new preresolvedDialer for
EOF
@@ -570,3 +570,5 @@

va.log.Debugf("following redirect to host %q url %q", req.Host, req.URL.String())
sanitizedURL := strings.ReplaceAll(req.URL.String(), "\n", "")
sanitizedURL = strings.ReplaceAll(sanitizedURL, "\r", "")
va.log.Debugf("following redirect to host %q url %q", req.Host, sanitizedURL)
// Replace the transport's DialContext with the new preresolvedDialer for
Copilot is powered by AI and may make mistakes. Always verify output.
) (*x509.Certificate, *tls.ConnectionState, error) {
va.log.Info(fmt.Sprintf("%s [%s] Attempting to validate for %s %s", core.ChallengeTypeTLSALPN01, identifier, hostPort, config.ServerName))
// We expect a self-signed challenge certificate, do not verify it here.
config.InsecureSkipVerify = true

Check failure

Code scanning / CodeQL

Disabled TLS certificate check

InsecureSkipVerify should not be used in production code.

Copilot Autofix

AI about 1 year ago

To fix the issue, we need to replace the use of InsecureSkipVerify = true with a safer alternative. Instead of disabling verification entirely, we can implement a custom certificate verification mechanism using the VerifyPeerCertificate callback in the tls.Config structure. This allows us to validate the self-signed challenge certificate explicitly while maintaining security.

The changes will involve:

  1. Removing the InsecureSkipVerify = true assignment.
  2. Adding a custom VerifyPeerCertificate function to validate the self-signed certificate.

Suggested changeset 1
third-party/github.com/letsencrypt/boulder/va/tlsalpn.go

Autofix patch

Autofix patch
Run the following command in your local git repository to apply this patch
cat << 'EOF' | git apply
diff --git a/third-party/github.com/letsencrypt/boulder/va/tlsalpn.go b/third-party/github.com/letsencrypt/boulder/va/tlsalpn.go
--- a/third-party/github.com/letsencrypt/boulder/va/tlsalpn.go
+++ b/third-party/github.com/letsencrypt/boulder/va/tlsalpn.go
@@ -127,4 +127,18 @@
 	va.log.Info(fmt.Sprintf("%s [%s] Attempting to validate for %s %s", core.ChallengeTypeTLSALPN01, identifier, hostPort, config.ServerName))
-	// We expect a self-signed challenge certificate, do not verify it here.
-	config.InsecureSkipVerify = true
+	// We expect a self-signed challenge certificate, validate it using a custom verification mechanism.
+	config.VerifyPeerCertificate = func(rawCerts [][]byte, verifiedChains [][]*x509.Certificate) error {
+		if len(rawCerts) == 0 {
+			return errors.New("no certificates presented")
+		}
+		cert, err := x509.ParseCertificate(rawCerts[0])
+		if err != nil {
+			return fmt.Errorf("failed to parse certificate: %v", err)
+		}
+		// Perform custom validation logic for the self-signed certificate.
+		// Example: Check specific fields or constraints in the certificate.
+		if cert.Subject.CommonName != config.ServerName {
+			return fmt.Errorf("certificate CommonName %s does not match ServerName %s", cert.Subject.CommonName, config.ServerName)
+		}
+		return nil
+	}
 
EOF
@@ -127,4 +127,18 @@
va.log.Info(fmt.Sprintf("%s [%s] Attempting to validate for %s %s", core.ChallengeTypeTLSALPN01, identifier, hostPort, config.ServerName))
// We expect a self-signed challenge certificate, do not verify it here.
config.InsecureSkipVerify = true
// We expect a self-signed challenge certificate, validate it using a custom verification mechanism.
config.VerifyPeerCertificate = func(rawCerts [][]byte, verifiedChains [][]*x509.Certificate) error {
if len(rawCerts) == 0 {
return errors.New("no certificates presented")
}
cert, err := x509.ParseCertificate(rawCerts[0])
if err != nil {
return fmt.Errorf("failed to parse certificate: %v", err)
}
// Perform custom validation logic for the self-signed certificate.
// Example: Check specific fields or constraints in the certificate.
if cert.Subject.CommonName != config.ServerName {
return fmt.Errorf("certificate CommonName %s does not match ServerName %s", cert.Subject.CommonName, config.ServerName)
}
return nil
}

Copilot is powered by AI and may make mistakes. Always verify output.
}
th.log.Infof("%s %s %d %d %d %s JSON=%s",
logEvent.Method, logEvent.Endpoint, logEvent.Requester, logEvent.Code,
int(logEvent.Latency*1000), logEvent.RealIP, jsonEvent)

Check failure

Code scanning / CodeQL

Log entries created from user input

This log entry depends on a [user-provided value](1).

Copilot Autofix

AI about 1 year ago

To fix the issue, we need to sanitize the realIP value before it is logged. Specifically:

  1. Remove any newline (\n) or carriage return (\r) characters from the realIP value using strings.ReplaceAll.
  2. Ensure that the sanitized value is used in the logEvent.RealIP field.

The changes will be made in the ServeHTTP method of the TopHandler struct, where the realIP value is assigned to logEvent.RealIP.

Suggested changeset 1
third-party/github.com/letsencrypt/boulder/web/context.go

Autofix patch

Autofix patch
Run the following command in your local git repository to apply this patch
cat << 'EOF' | git apply
diff --git a/third-party/github.com/letsencrypt/boulder/web/context.go b/third-party/github.com/letsencrypt/boulder/web/context.go
--- a/third-party/github.com/letsencrypt/boulder/web/context.go
+++ b/third-party/github.com/letsencrypt/boulder/web/context.go
@@ -120,2 +120,6 @@
 		realIP = "0.0.0.0"
+	} else {
+		// Sanitize the realIP value to remove newline and carriage return characters
+		realIP = strings.ReplaceAll(realIP, "\n", "")
+		realIP = strings.ReplaceAll(realIP, "\r", "")
 	}
EOF
@@ -120,2 +120,6 @@
realIP = "0.0.0.0"
} else {
// Sanitize the realIP value to remove newline and carriage return characters
realIP = strings.ReplaceAll(realIP, "\n", "")
realIP = strings.ReplaceAll(realIP, "\r", "")
}
Copilot is powered by AI and may make mistakes. Always verify output.
Comment thread docs/license-compliance.md
Comment thread script/licenses-check Outdated
With these changes, `cli/cli` will be redistributing code as-is due to license compliance, which we will not change or address issues around.  Without these changes, our pull requests are getting a bunch of false positive annotations we cannot and will not fix directly.
@andyfeller
andyfeller requested a review from BagToad June 20, 2025 20:55
@vovong92

This comment was marked as spam.

@BagToad BagToad left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

LGTM! I don't think anything is blocking enough to avoid merge this 👏

I would like to understand the versioning strategy a bit more, but I don't think that blocks this PR - more details in my comment on script/licenses :)

Comment thread script/licenses
@@ -0,0 +1,25 @@
#!/bin/bash

go install github.com/google/go-licenses@latest

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Question: Are we comfortable with always using the latest here, or should we pin to a sha for stability/security etc?

I know that pinning comes with the "when does this get updated then?" question, and I don't know the answer to that either :P

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I guess this is especially in my mind with this script being used in actions... 🤔

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

@BagToad : what do you think about conditionalizing this to skip installing this if CI, leaving that to the workflow?

otherwise, everything running the script has to manage its installation. conditionalizing it will make it easy for contributors and maintainers.

Comment thread script/licenses
Comment on lines +9 to +12
# Clear third-party source code to avoid stale content
rm -rf third-party
mkdir -p third-party

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Question: Should we check successes (exit codes) here? Are we okay with this maybe failing but then continuing?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I'm not worried about these commands failing, but if there is a concern then we might want to set -e so any errors cause the whole script to fail.

Comment thread script/licenses

# Setup temporary directory to collect updated third-party source code
export TEMPDIR="$(mktemp -d)"
trap "rm -fr ${TEMPDIR}" EXIT

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

(praise) Smart!

Comment thread script/licenses
echo "Generating licenses for ${goos}..."
GOOS="${goos}" go-licenses save ./... --save_path="${TEMPDIR}/${goos}" --force || echo "Ignore warnings"
GOOS="${goos}" go-licenses report ./... --template .github/licenses.tmpl --ignore github.com/cli/cli > third-party-licenses.${goos}.md || echo "Ignore warnings"
cp -fR "${TEMPDIR}/${goos}"/* third-party/

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Question: Similar question here - should we check exit codes here or keep going even if something failed?

@andyfeller
andyfeller merged commit f721856 into trunk Jun 23, 2025
@andyfeller
andyfeller deleted the andyfeller/9422-license-compliance branch June 23, 2025 16:06
andyfeller added a commit that referenced this pull request Jul 23, 2025
Fixes #11270

This commit refactors the work done in #11047 of blocking pull requests for manual `third-party` license updates to having GitHub Actions automatically update it on pushes to `trunk`.

This will allow maintainers to streamline Dependabot PR reviews while reducing contributor friction when changing dependencies.
tmeijn pushed a commit to tmeijn/dotfiles that referenced this pull request Jul 27, 2025
This MR contains the following updates:

| Package | Update | Change |
|---|---|---|
| [cli/cli](https://github.com/cli/cli) | minor | `v2.74.2` -> `v2.76.1` |

MR created with the help of [el-capitano/tools/renovate-bot](https://gitlab.com/el-capitano/tools/renovate-bot).

**Proposed changes to behavior should be submitted there as MRs.**

---

### Release Notes

<details>
<summary>cli/cli (cli/cli)</summary>

### [`v2.76.1`](https://github.com/cli/cli/releases/tag/v2.76.1): GitHub CLI 2.76.1

[Compare Source](cli/cli@v2.76.0...v2.76.1)

#### `gh pr create` regression fix

This release fixes a regression introduced in `v2.76.0` where organization teams were retrieved outside of intentional use cases.  This caused problems for GitHub Enterprise Server users using the GitHub Actions automatic token that does not have access to organization teams.

For more information, see cli/cli#11360

#### What's Changed

##### 🐛 Fixes

- Fix: `gh pr create`, only fetch teams when reviewers contain a team  by [@&#8203;BagToad](https://github.com/BagToad) in cli/cli#11361

##### 📚 Docs & Chores

- add tenancy aware for san matcher by [@&#8203;ejahnGithub](https://github.com/ejahnGithub) in cli/cli#11261
- Run Lint and Tests on `push` to `trunk` branch by [@&#8203;andyfeller](https://github.com/andyfeller) in cli/cli#11325
- update ownership of pkg/cmd/release/shared/ by [@&#8203;ejahnGithub](https://github.com/ejahnGithub) in cli/cli#11326
- Automate spam issue detection by [@&#8203;babakks](https://github.com/babakks) in cli/cli#11316
- Improve `api` `--preview` docs by [@&#8203;jsoref](https://github.com/jsoref) in cli/cli#11274
- Incorporate govulncheck into workflows by [@&#8203;andyfeller](https://github.com/andyfeller) in cli/cli#11332
- chore(deps): bump advanced-security/filter-sarif from 1.0.0 to 1.0.1 by [@&#8203;dependabot](https://github.com/dependabot)\[bot] in cli/cli#11298
- chore(deps): bump github.com/sigstore/sigstore-go from 1.0.0 to 1.1.0 by [@&#8203;dependabot](https://github.com/dependabot)\[bot] in cli/cli#11307

**Full Changelog**: cli/cli@v2.76.0...v2.76.1

### [`v2.76.0`](https://github.com/cli/cli/releases/tag/v2.76.0): GitHub CLI 2.76.0

[Compare Source](cli/cli@v2.75.1...v2.76.0)

#### :copilot: Copilot Coding Agent Support

GitHub Copilot Pro+ and Copilot Enterprise subscribers can now assign issues to GitHub Copilot during issue creation using:

- Command-line flag: `gh issue create --assignee @&#8203;copilot`
- Launching web browser: `gh issue create --assignee @&#8203;copilot --web`
- Or interactively selecting `Copilot (AI)` as assignee in `gh issue create` metadata

For more details, refer to [the full changelog post for Copilot coding agent](https://github.blog/changelog/2025-05-19-github-copilot-coding-agent-in-public-preview/).

#### What's Changed

##### ✨ Features

- Assign Copilot during `gh issue create` by [@&#8203;andyfeller](https://github.com/andyfeller) in cli/cli#11279
- Display immutable field in `release view` command by [@&#8203;bdehamer](https://github.com/bdehamer) in cli/cli#11251

##### 🐛 Fixes

- FIX: Do not fetch logs for skipped jobs by [@&#8203;babakks](https://github.com/babakks) in cli/cli#11312
- Transform `extension` and `filename` qualifiers into `path` qualifier for web code search by [@&#8203;samcoe](https://github.com/samcoe) in cli/cli#11211

##### 📚 Docs & Chores

- FIX: Workflow does not contain permissions by [@&#8203;BagToad](https://github.com/BagToad) in cli/cli#11322
- Add automated feature request response workflow by [@&#8203;BagToad](https://github.com/BagToad) in cli/cli#11299

**Full Changelog**: cli/cli@v2.75.1...v2.76.0

### [`v2.75.1`](https://github.com/cli/cli/releases/tag/v2.75.1): GitHub CLI 2.75.1

[Compare Source](cli/cli@v2.75.0...v2.75.1)

#### What's Changed

##### 🐛 Fixes

- Ensure hostnames are visible in CLI website by [@&#8203;andyfeller](https://github.com/andyfeller) in cli/cli#11295
- Revert "Fix: `gh pr create` prioritize `--title` and `--body` over `--fill` when `--web` is present" by [@&#8203;andyfeller](https://github.com/andyfeller) in cli/cli#11300

##### 📚 Docs & Chores

- Ensure go directive is always .0 version in bump by [@&#8203;williammartin](https://github.com/williammartin) in cli/cli#11259
- Minor (1-word) documentation typo in generated `~/.config/gh/config.yml` by [@&#8203;kurahaupo](https://github.com/kurahaupo) in cli/cli#11246
- Automate closing of stale issues by [@&#8203;babakks](https://github.com/babakks) in cli/cli#11268
- Filter the `third-party/` folder out of CodeQL results by [@&#8203;BagToad](https://github.com/BagToad) in cli/cli#11278
- Exclude `third-party` source from golangci-lint by [@&#8203;andyfeller](https://github.com/andyfeller) in cli/cli#11293

##### :dependabot: Dependencies

- Bump Go to 1.24.5 by [@&#8203;github-actions](https://github.com/github-actions)\[bot] in cli/cli#11255
- chore(deps): bump github.com/sigstore/protobuf-specs from 0.4.3 to 0.5.0 by [@&#8203;dependabot](https://github.com/dependabot)\[bot] in cli/cli#11263
- chore(deps): bump golang.org/x/term from 0.32.0 to 0.33.0 by [@&#8203;dependabot](https://github.com/dependabot)\[bot] in cli/cli#11266
- chore(deps): bump golang.org/x/sync from 0.15.0 to 0.16.0 by [@&#8203;dependabot](https://github.com/dependabot)\[bot] in cli/cli#11264
- chore(deps): bump golang.org/x/text from 0.26.0 to 0.27.0 by [@&#8203;dependabot](https://github.com/dependabot)\[bot] in cli/cli#11265
- chore(deps): bump golang.org/x/crypto from 0.39.0 to 0.40.0 by [@&#8203;dependabot](https://github.com/dependabot)\[bot] in cli/cli#11275

#### New Contributors

- [@&#8203;kurahaupo](https://github.com/kurahaupo) made their first contribution in cli/cli#11246
- [@&#8203;github-actions](https://github.com/github-actions)\[bot] made their first contribution in cli/cli#11255

**Full Changelog**: cli/cli@v2.75.0...v2.75.1

### [`v2.75.0`](https://github.com/cli/cli/releases/tag/v2.75.0): GitHub CLI 2.75.0

[Compare Source](cli/cli@v2.74.2...v2.75.0)

#### What's Changed

##### ✨ Features

- init release verify subcommands  by [@&#8203;ejahnGithub](https://github.com/ejahnGithub) in cli/cli#11018
- Embed Windows resources (VERSIONINFO) during build by [@&#8203;babakks](https://github.com/babakks) in cli/cli#11048
- Support `--no-repos-selected` on `gh secret set` by [@&#8203;williammartin](https://github.com/williammartin) in cli/cli#11217

##### 🐛 Fixes

- Fix: `gh pr create` prioritize `--title` and `--body` over `--fill` when `--web` is present by [@&#8203;dankrzeminski32](https://github.com/dankrzeminski32) in cli/cli#10547
- fix: get token for active user instead of blank if possible by [@&#8203;anuraaga](https://github.com/anuraaga) in cli/cli#11038
- Use Actions API to retrieve job run logs as a fallback mechanism  by [@&#8203;babakks](https://github.com/babakks) in cli/cli#11172
- Fix query object state mutation during pagination by [@&#8203;babakks](https://github.com/babakks) in cli/cli#11244
- Handle `HTTP 404` when deleting remote branch in `pr merge` by [@&#8203;babakks](https://github.com/babakks) in cli/cli#11234

##### 📚 Docs & Chores

- chore: fix function name by [@&#8203;jinjingroad](https://github.com/jinjingroad) in cli/cli#11149
- chore: update Go version to 1.24 in devcontainer configuration and docs by [@&#8203;tMinamiii](https://github.com/tMinamiii) in cli/cli#11158
- Ensure lint workflow checks whether 3rd party license and code is up to date by [@&#8203;andyfeller](https://github.com/andyfeller) in cli/cli#11047
- docs: install\_linux.md: add Solus linux install instructions by [@&#8203;chax](https://github.com/chax) in cli/cli#10823
- Fix missing newline in install\_linux.md by [@&#8203;BagToad](https://github.com/BagToad) in cli/cli#11160
- Ensure automation uses pinned go-licenses version by [@&#8203;andyfeller](https://github.com/andyfeller) in cli/cli#11161
- Add `workflow_dispatch` support to MR Help Wanted check by [@&#8203;BagToad](https://github.com/BagToad) in cli/cli#11179
- Remove unused `GH_TOKEN` env variable from workflow by [@&#8203;BagToad](https://github.com/BagToad) in cli/cli#11190
- Add workflow to automate go version bumping by [@&#8203;williammartin](https://github.com/williammartin) in cli/cli#11189
- Fix inconsistent use of tabs and spaces by [@&#8203;Stefan-Heimersheim](https://github.com/Stefan-Heimersheim) in cli/cli#11194
- Decouple arg parsing from MR finder by [@&#8203;babakks](https://github.com/babakks) in cli/cli#11192
- docs: consistently use `apt` in installation instructions by [@&#8203;tklauser](https://github.com/tklauser) in cli/cli#11216
- Ensure bump go script has git user configured by [@&#8203;williammartin](https://github.com/williammartin) in cli/cli#11229
- Inject token into bump-go workflow by [@&#8203;williammartin](https://github.com/williammartin) in cli/cli#11233
- Reinstating Primer Style CLI content within `cli/cli` repository by [@&#8203;andyfeller](https://github.com/andyfeller) in cli/cli#11060
- Add setup-go to bump-go workflow by [@&#8203;williammartin](https://github.com/williammartin) in cli/cli#11237
- Ensure GoReleaser does not break on Mac OS and Linux when skipping Windows `.rsyso` generation script by [@&#8203;andyfeller](https://github.com/andyfeller) in cli/cli#11257

##### :dependabot: Dependencies

- Bump all dependencies except dev-tunnels by [@&#8203;williammartin](https://github.com/williammartin) in cli/cli#11203
- Update microsoft dev-tunnels to v0.1.13 by [@&#8203;williammartin](https://github.com/williammartin) in cli/cli#11205
- Consume dependabot minor versions for go modules by [@&#8203;williammartin](https://github.com/williammartin) in cli/cli#11213

#### New Contributors

- [@&#8203;jinjingroad](https://github.com/jinjingroad) made their first contribution in cli/cli#11149
- [@&#8203;tMinamiii](https://github.com/tMinamiii) made their first contribution in cli/cli#11158
- [@&#8203;chax](https://github.com/chax) made their first contribution in cli/cli#10823
- [@&#8203;dankrzeminski32](https://github.com/dankrzeminski32) made their first contribution in cli/cli#10547
- [@&#8203;anuraaga](https://github.com/anuraaga) made their first contribution in cli/cli#11038
- [@&#8203;Stefan-Heimersheim](https://github.com/Stefan-Heimersheim) made their first contribution in cli/cli#11194

**Full Changelog**: cli/cli@v2.74.2...v2.75.0

</details>

---

### Configuration

📅 **Schedule**: Branch creation - At any time (no schedule defined), Automerge - At any time (no schedule defined).

🚦 **Automerge**: Disabled by config. Please merge this manually once you are satisfied.

♻ **Rebasing**: Whenever MR becomes conflicted, or you tick the rebase/retry checkbox.

🔕 **Ignore**: Close this MR and you won't be reminded about this update again.

---

 - [ ] <!-- rebase-check -->If you want to rebase/retry this MR, check this box

---

This MR has been generated by [Renovate Bot](https://github.com/renovatebot/renovate).
<!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiI0MC42Mi4xIiwidXBkYXRlZEluVmVyIjoiNDAuNjIuMSIsInRhcmdldEJyYW5jaCI6Im1haW4iLCJsYWJlbHMiOlsiUmVub3ZhdGUgQm90Il19-->
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.

Apache 2.0 usage without preservation of copyright and license notices

9 participants