-
Notifications
You must be signed in to change notification settings - Fork 457
Implementing the Infinity data source patterns as an Alloy component. #4555
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
base: main
Are you sure you want to change the base?
Conversation
| Message: err.Error(), | ||
| UpdateTime: time.Now(), | ||
| }) | ||
| level.Error(c.opts.Logger).Log("msg", "infinity component query failed", "status", status, "output", output, "error", err) |
Check failure
Code scanning / CodeQL
Log entries created from user input High
user-provided value
This log entry depends on a
user-provided value
This log entry depends on a
user-provided value
This log entry depends on a
user-provided value
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI about 1 month ago
To fix this issue, we should sanitize user-controlled values before logging them. Specifically, in log entries that include error values that may incorporate unsanitized user input (such as URLs), we should strip newline (\n) and carriage return (\r) characters and possibly clearly delimit the user input in logs. The main field of concern here is the err value that is logged on line 729 in function Run.
The best way to fix this, according to the recommendation, is to sanitize err.Error() to remove line breaks and carriage returns before passing it to the logger. However, level.Error(...).Log(...) is a structured logger and the error is passed as an object; so it is better to convert the error to string, sanitize it, and log the sanitized error string instead of passing the raw error object.
Specifically, on line 729, replace "error", err with "error", sanitizedErrMsg, where sanitizedErrMsg is the error string with all \n and \r characters removed.
You will need to:
- Before calling the logger, convert
errto a string (err.Error()) and sanitize it withstrings.ReplaceAlltwice (remove\nand\r). - Use the sanitized string in the log call.
Importantly, don't change the existing log structure, just sanitize the logged error string.
-
Copy modified lines R729-R730
| @@ -726,7 +726,8 @@ | ||
| Message: err.Error(), | ||
| UpdateTime: time.Now(), | ||
| }) | ||
| level.Error(c.opts.Logger).Log("msg", "infinity component query failed", "status", status, "output", output, "error", err) | ||
| sanitizedErrMsg := strings.ReplaceAll(strings.ReplaceAll(err.Error(), "\n", "",), "\r", "") | ||
| level.Error(c.opts.Logger).Log("msg", "infinity component query failed", "status", status, "output", output, "error", sanitizedErrMsg) | ||
| c.mut.Unlock() | ||
| continue | ||
| } |
| //logger.Debug("requesting URL", "host", req.URL.Hostname(), "url_path", req.URL.Path, "method", req.Method, "type", query.Type) | ||
| res, err := c.client.Do(req) | ||
| duration := time.Since(startTime) | ||
| level.Debug(c.opts.Logger).Log("msg", "received response", "host", req.URL.Hostname(), "url_path", req.URL.Path, "method", req.Method, "type", c.infinityQuery.Type, "duration_ms", duration.Milliseconds()) |
Check failure
Code scanning / CodeQL
Log entries created from user input High
user-provided value
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI about 1 month ago
To correct the issue, you should sanitize (escape) the value of req.URL.Hostname() before logging. Since the log output is plain text, you need to remove or escape any carriage return (\r), newline (\n), or other control characters that could allow a malicious user to manipulate the log. The recommended approach is to use strings.ReplaceAll to remove any newline or carriage return characters in the hostname extracted from the untrusted URL. This should be performed just before the value is passed to the log statement.
You should only sanitize the values that can be influenced by user input, so apply this to other untrusted values if present. For this fix, focus on replacing line 880 so that "host" receives the sanitized hostname. You need to ensure that strings is imported (it already is in the code), so no new import is required.
-
Copy modified lines R880-R881
| @@ -877,7 +877,8 @@ | ||
| //logger.Debug("requesting URL", "host", req.URL.Hostname(), "url_path", req.URL.Path, "method", req.Method, "type", query.Type) | ||
| res, err := c.client.Do(req) | ||
| duration := time.Since(startTime) | ||
| level.Debug(c.opts.Logger).Log("msg", "received response", "host", req.URL.Hostname(), "url_path", req.URL.Path, "method", req.Method, "type", c.infinityQuery.Type, "duration_ms", duration.Milliseconds()) | ||
| safeHostname := strings.ReplaceAll(strings.ReplaceAll(req.URL.Hostname(), "\n", ""), "\r", "") | ||
| level.Debug(c.opts.Logger).Log("msg", "received response", "host", safeHostname, "url_path", req.URL.Path, "method", req.Method, "type", c.infinityQuery.Type, "duration_ms", duration.Milliseconds()) | ||
| if res != nil { | ||
| defer func() { | ||
| if err := res.Body.Close(); err != nil { |
| //logger.Debug("requesting URL", "host", req.URL.Hostname(), "url_path", req.URL.Path, "method", req.Method, "type", query.Type) | ||
| res, err := c.client.Do(req) | ||
| duration := time.Since(startTime) | ||
| level.Debug(c.opts.Logger).Log("msg", "received response", "host", req.URL.Hostname(), "url_path", req.URL.Path, "method", req.Method, "type", c.infinityQuery.Type, "duration_ms", duration.Milliseconds()) |
Check failure
Code scanning / CodeQL
Log entries created from user input High
user-provided value
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI about 1 month ago
To fix the problem, sanitize the value of req.URL.Path before including it in the log. Since logs are likely plain text, remove (or replace) any newline (\n) and carriage return (\r) characters from the string. The best fix is to define a local variable, e.g. escapedPath, which is the req.URL.Path with all such characters removed (using strings.ReplaceAll). Then, log the sanitized path value in place of the original.
All edits are in the file internal/component/infinity/infinity.go, specifically line 880 in getHTTPResponse.
-
Copy modified lines R880-R882
| @@ -877,7 +877,9 @@ | ||
| //logger.Debug("requesting URL", "host", req.URL.Hostname(), "url_path", req.URL.Path, "method", req.Method, "type", query.Type) | ||
| res, err := c.client.Do(req) | ||
| duration := time.Since(startTime) | ||
| level.Debug(c.opts.Logger).Log("msg", "received response", "host", req.URL.Hostname(), "url_path", req.URL.Path, "method", req.Method, "type", c.infinityQuery.Type, "duration_ms", duration.Milliseconds()) | ||
| escapedPath := strings.ReplaceAll(req.URL.Path, "\n", "") | ||
| escapedPath = strings.ReplaceAll(escapedPath, "\r", "") | ||
| level.Debug(c.opts.Logger).Log("msg", "received response", "host", req.URL.Hostname(), "url_path", escapedPath, "method", req.Method, "type", c.infinityQuery.Type, "duration_ms", duration.Milliseconds()) | ||
| if res != nil { | ||
| defer func() { | ||
| if err := res.Body.Close(); err != nil { |
| } | ||
| if err != nil { | ||
| if res != nil { | ||
| level.Debug(c.opts.Logger).Log("msg", "error getting response from server", "url", req.URL.String(), "method", req.Method, "error", err.Error(), "status code", res.StatusCode) |
Check failure
Code scanning / CodeQL
Log entries created from user input High
user-provided value
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI about 1 month ago
To prevent the possibility of log-forgery via malicious user input, we should sanitize or encode any user-controlled values before logging. For plain text logs, we should remove any carriage return (\r) and line-feed (\n) characters from these values. The URLs and errors logged in this function should have these characters stripped before being passed to the log function. This can be implemented by defining a small helper (e.g., sanitizeLogString(s string) string) that does this, then wrapping every use of req.URL.String() (and any other user-controlled value as necessary before logging) in the sanitizer.
Within internal/component/infinity/infinity.go, add the helper function, and update all log invocations in getHTTPResponse that use req.URL.String() to sanitize the string first.
-
Copy modified lines R40-R47 -
Copy modified line R898 -
Copy modified line R901 -
Copy modified line R904 -
Copy modified lines R907-R908 -
Copy modified lines R911-R912 -
Copy modified line R917
| @@ -37,6 +37,14 @@ | ||
| backendLog "github.com/grafana/grafana-plugin-sdk-go/backend/log" | ||
| ) | ||
|
|
||
|
|
||
| // sanitizeLogString removes CR and LF characters from input to prevent log-forgery. | ||
| func sanitizeLogString(s string) string { | ||
| s = strings.ReplaceAll(s, "\n", "") | ||
| s = strings.ReplaceAll(s, "\r", "") | ||
| return s | ||
| } | ||
|
|
||
| func init() { | ||
| component.Register(component.Registration{ | ||
| Name: "infinity", | ||
| @@ -887,21 +895,21 @@ | ||
| } | ||
| if err != nil { | ||
| if res != nil { | ||
| level.Debug(c.opts.Logger).Log("msg", "error getting response from server", "url", req.URL.String(), "method", req.Method, "error", err.Error(), "status code", res.StatusCode) | ||
| level.Debug(c.opts.Logger).Log("msg", "error getting response from server", "url", sanitizeLogString(req.URL.String()), "method", req.Method, "error", err.Error(), "status code", res.StatusCode) | ||
| // Infinity can query anything and users are responsible for ensuring that endpoint/auth is correct | ||
| // therefore any incoming error is considered downstream | ||
| return nil, res.StatusCode, fmt.Errorf("error getting response from %s", req.URL.String()) | ||
| return nil, res.StatusCode, fmt.Errorf("error getting response from %s", sanitizeLogString(req.URL.String())) | ||
| } | ||
| if errors.Is(err, context.Canceled) { | ||
| level.Debug(c.opts.Logger).Log("msg", "request cancelled", "url", req.URL.String(), "method", req.Method) | ||
| level.Debug(c.opts.Logger).Log("msg", "request cancelled", "url", sanitizeLogString(req.URL.String()), "method", req.Method) | ||
| return nil, http.StatusInternalServerError, err | ||
| } | ||
| level.Debug(c.opts.Logger).Log("msg", "error getting response from server. no response received", "url", req.URL.String(), "error", err.Error()) | ||
| return nil, http.StatusInternalServerError, fmt.Errorf("error getting response from url %s. no response received. Error: %w", req.URL.String(), err) | ||
| level.Debug(c.opts.Logger).Log("msg", "error getting response from server. no response received", "url", sanitizeLogString(req.URL.String()), "error", err.Error()) | ||
| return nil, http.StatusInternalServerError, fmt.Errorf("error getting response from url %s. no response received. Error: %w", sanitizeLogString(req.URL.String()), err) | ||
| } | ||
| if res == nil { | ||
| level.Debug(c.opts.Logger).Log("msg", "invalid response from server and also no error", "url", req.URL.String(), "method", req.Method) | ||
| return nil, http.StatusInternalServerError, fmt.Errorf("invalid response received for the URL %s", req.URL.String()) | ||
| level.Debug(c.opts.Logger).Log("msg", "invalid response from server and also no error", "url", sanitizeLogString(req.URL.String()), "method", req.Method) | ||
| return nil, http.StatusInternalServerError, fmt.Errorf("invalid response received for the URL %s", sanitizeLogString(req.URL.String())) | ||
| } | ||
| if res.StatusCode >= http.StatusBadRequest && !c.infinitySettings.IgnoreStatusCodeCheck { | ||
| err = fmt.Errorf("%w\nstatus code : %s", models.ErrUnsuccessfulHTTPResponseStatus, res.Status) | ||
| @@ -914,7 +914,7 @@ | ||
| return nil, res.StatusCode, err | ||
| } | ||
| if len(bodyBytes) == 0 { | ||
| return nil, res.StatusCode, fmt.Errorf("empty response body received for the URL %s", req.URL.String()) | ||
| return nil, res.StatusCode, fmt.Errorf("empty response body received for the URL %s", sanitizeLogString(req.URL.String())) | ||
| } | ||
| bodyBytes = removeBOMContent(bodyBytes) | ||
| if ds.CanParseAsJSON(c.infinityQuery.Type, res.Header) { |
| return nil, res.StatusCode, fmt.Errorf("error getting response from %s", req.URL.String()) | ||
| } | ||
| if errors.Is(err, context.Canceled) { | ||
| level.Debug(c.opts.Logger).Log("msg", "request cancelled", "url", req.URL.String(), "method", req.Method) |
Check failure
Code scanning / CodeQL
Log entries created from user input High
user-provided value
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI about 1 month ago
To fix this issue, we should sanitize user input before logging it. Specifically, when logging user-controlled strings (such as a full URL), we should remove or encode newlines and carriage returns to prevent log forging. The best approach is to remove all \n and \r characters from the logged string. We can do this using strings.ReplaceAll to strip these control characters from req.URL.String() prior to logging.
In the block at line 896, we should replace:
level.Debug(c.opts.Logger).Log("msg", "request cancelled", "url", req.URL.String(), "method", req.Method)with
urlToLog := strings.ReplaceAll(req.URL.String(), "\n", "")
urlToLog = strings.ReplaceAll(urlToLog, "\r", "")
level.Debug(c.opts.Logger).Log("msg", "request cancelled", "url", urlToLog, "method", req.Method)Because we need access to the strings package (which is already imported), no import changes are necessary.
-
Copy modified lines R896-R898
| @@ -893,7 +893,9 @@ | ||
| return nil, res.StatusCode, fmt.Errorf("error getting response from %s", req.URL.String()) | ||
| } | ||
| if errors.Is(err, context.Canceled) { | ||
| level.Debug(c.opts.Logger).Log("msg", "request cancelled", "url", req.URL.String(), "method", req.Method) | ||
| urlToLog := strings.ReplaceAll(req.URL.String(), "\n", "") | ||
| urlToLog = strings.ReplaceAll(urlToLog, "\r", "") | ||
| level.Debug(c.opts.Logger).Log("msg", "request cancelled", "url", urlToLog, "method", req.Method) | ||
| return nil, http.StatusInternalServerError, err | ||
| } | ||
| level.Debug(c.opts.Logger).Log("msg", "error getting response from server. no response received", "url", req.URL.String(), "error", err.Error()) |
| level.Debug(c.opts.Logger).Log("msg", "request cancelled", "url", req.URL.String(), "method", req.Method) | ||
| return nil, http.StatusInternalServerError, err | ||
| } | ||
| level.Debug(c.opts.Logger).Log("msg", "error getting response from server. no response received", "url", req.URL.String(), "error", err.Error()) |
Check failure
Code scanning / CodeQL
Log entries created from user input High
user-provided value
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI about 1 month ago
To fix the problem, sanitize the user-supplied URL before logging it. Since the log output is in plain text (not HTML), the recommended mitigation is to strip out potentially dangerous characters from the URL string—specifically, newline and carriage return characters. strings.ReplaceAll can be used for this. You should perform the sanitization immediately before logging any user data, or (for cleanliness and DRYness), define a helper function for sanitizing input for logs. Only internal/component/infinity/infinity.go, within the lines shown, must be updated. Add a small helper function, if desired, within the file for clarity and reuse.
You will need to add the helper function (e.g., sanitizeForLog), its import (if necessary), and update the affected usages of req.URL.String() by passing them through the sanitizer before logging.
-
Copy modified lines R40-R47 -
Copy modified line R907 -
Copy modified line R911
| @@ -37,6 +37,14 @@ | ||
| backendLog "github.com/grafana/grafana-plugin-sdk-go/backend/log" | ||
| ) | ||
|
|
||
|
|
||
| // sanitizeForLog removes line breaks to prevent log forging via user-controlled input. | ||
| func sanitizeForLog(input string) string { | ||
| s := strings.ReplaceAll(input, "\n", "") | ||
| s = strings.ReplaceAll(s, "\r", "") | ||
| return s | ||
| } | ||
|
|
||
| func init() { | ||
| component.Register(component.Registration{ | ||
| Name: "infinity", | ||
| @@ -896,11 +904,11 @@ | ||
| level.Debug(c.opts.Logger).Log("msg", "request cancelled", "url", req.URL.String(), "method", req.Method) | ||
| return nil, http.StatusInternalServerError, err | ||
| } | ||
| level.Debug(c.opts.Logger).Log("msg", "error getting response from server. no response received", "url", req.URL.String(), "error", err.Error()) | ||
| level.Debug(c.opts.Logger).Log("msg", "error getting response from server. no response received", "url", sanitizeForLog(req.URL.String()), "error", err.Error()) | ||
| return nil, http.StatusInternalServerError, fmt.Errorf("error getting response from url %s. no response received. Error: %w", req.URL.String(), err) | ||
| } | ||
| if res == nil { | ||
| level.Debug(c.opts.Logger).Log("msg", "invalid response from server and also no error", "url", req.URL.String(), "method", req.Method) | ||
| level.Debug(c.opts.Logger).Log("msg", "invalid response from server and also no error", "url", sanitizeForLog(req.URL.String()), "method", req.Method) | ||
| return nil, http.StatusInternalServerError, fmt.Errorf("invalid response received for the URL %s", req.URL.String()) | ||
| } | ||
| if res.StatusCode >= http.StatusBadRequest && !c.infinitySettings.IgnoreStatusCodeCheck { |
| return nil, http.StatusInternalServerError, fmt.Errorf("error getting response from url %s. no response received. Error: %w", req.URL.String(), err) | ||
| } | ||
| if res == nil { | ||
| level.Debug(c.opts.Logger).Log("msg", "invalid response from server and also no error", "url", req.URL.String(), "method", req.Method) |
Check failure
Code scanning / CodeQL
Log entries created from user input High
user-provided value
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI about 1 month ago
The correct fix is to sanitize the user-provided value before logging it. Since the logs in question are plain text, follow the recommendation: strip all newline (\n) and carriage return (\r) characters from the string before passing it to the log function. Edit line 903 and any other log statements handling req.URL.String() in the visible code, applying sanitization via strings.ReplaceAll. Ensure the required import ("strings") is present—it's already imported. Only edit the relevant region (the arguments to the log statement) and preserve the existing functionality and log structure.
-
Copy modified lines R903-R905
| @@ -900,8 +900,9 @@ | ||
| return nil, http.StatusInternalServerError, fmt.Errorf("error getting response from url %s. no response received. Error: %w", req.URL.String(), err) | ||
| } | ||
| if res == nil { | ||
| level.Debug(c.opts.Logger).Log("msg", "invalid response from server and also no error", "url", req.URL.String(), "method", req.Method) | ||
| return nil, http.StatusInternalServerError, fmt.Errorf("invalid response received for the URL %s", req.URL.String()) | ||
| safeURL := strings.ReplaceAll(strings.ReplaceAll(req.URL.String(), "\n", ""), "\r", "") | ||
| level.Debug(c.opts.Logger).Log("msg", "invalid response from server and also no error", "url", safeURL, "method", req.Method) | ||
| return nil, http.StatusInternalServerError, fmt.Errorf("invalid response received for the URL %s", safeURL) | ||
| } | ||
| if res.StatusCode >= http.StatusBadRequest && !c.infinitySettings.IgnoreStatusCodeCheck { | ||
| err = fmt.Errorf("%w\nstatus code : %s", models.ErrUnsuccessfulHTTPResponseStatus, res.Status) |
This was a hackathon project, still quite a bit of work to get it production ready but it was an interesting exercise. Considering this as a potential way to solve issues where customers want to turn arbitrary JSON/etc into telemetry.