Redact URL paths in network request telemetry spans#85
Conversation
369b608 to
e0fe6ce
Compare
| ) | ||
| hostname := info.Profile.GetHostname() | ||
| if !info.Profile.IsHCPTerraform() { | ||
| if !info.Profile.IsHCPTerraform() && hostname != "archivist.terraform.io" && hostname != "archivist.eu.terraform.io" { |
There was a problem hiding this comment.
This might be a better condition:
!strings.HasSuffix("terraform.io")
There was a problem hiding this comment.
I like moving off the hardcoded list but it needs the two-arg form and without a dot boundary a wrong url could go through or if there's a port on the hostname, which GetHostname() allows. Something like:
func isKnownHCPHost(h string) bool {
h, _, _ = strings.Cut(h, ":")
return h == "terraform.io" || strings.HasSuffix(h, ".terraform.io")
}
and then if !isKnownHCPHost(hostname) { hostname = generateStableID(...) }, The main thing I want to keep is that unknown hosts still get hashed by default
| var ( | ||
| // Redactions should use capture groups to identify sensitive segments of a path, which | ||
| // will be replaced with a redaction placeholder. | ||
| redactions = []*regexp.Regexp{ |
There was a problem hiding this comment.
The module regex only redacts the namespace. I ran this against a realistic path and the module name and provider still leak:
/v1/modules/hashicorp/consul/aws/1.0.0/download → /v1/modules/<redacted>/consul/aws/1.0.0/download
Compare with the registry-providers rule just below, which correctly redacts two segments. For private modules the name/provider are customer data too so should this be /v1/modules/([^/]+)/([^/]+)/([^/]+) or similar?
| | http.duration_ms | 855 | Request latency | | ||
| | Key | Example Value | Source | | ||
| |---------------------|--------------------------|-------------------------------------| | ||
| | http.path | “/workspaces/<redacted>” | API request path, with IDs redacted | |
There was a problem hiding this comment.
A bare /api/v2/workspaces/ws-abc123 is not redacted only /organizations/{org}/workspaces/{ws} matches
| lastIndex := 0 | ||
|
|
||
| // indices[0] and indices[1] are the start/end of the FULL match. | ||
| // Capture groups start at index index 2 (indices[2] to indices[3] is Group 1, etc.) |
| // Write whatever remains of the path after the last redacted group | ||
| result.WriteString(path[lastIndex:]) | ||
|
|
||
| // Update path and return (or continue to next regex if paths can match multiple rules) |
There was a problem hiding this comment.
the comment says continue but the loop never returns early
2 participants
Description
Redact customer data from HTTP paths in telemetry payloads.
PR Checklist
npx changie newor install changie to prepare a new changelog entry for the next set of release notes.PCI review checklist
I have documented a clear reason for, and description of, the change I am making.
If applicable, I've documented a plan to revert these changes if they require more than reverting the pull request.
If applicable, I've documented the impact of any changes to security controls.
Examples of changes to security controls include using new access control methods, adding or removing logging pipelines, etc.