Skip to content

Commit 4fd87dc

Browse files
authored
fix(oauth): redact the whole of a base64 Bearer token (#1785)
## Bug `tokenPattern` in `internal/oauth/logging.go` is `(?i)(bearer\s+)[a-zA-Z0-9\-_\.]+`. RFC 6750's `b64token` also allows `~`, `+`, `/` and trailing `=`, and opaque tokens encoded in standard base64 contain `+` and `/`. The match stops at the first one, so only the head of the token is replaced and the rest stays in the text: ``` Bearer dGhpcy1pcy1h+c2VjcmV0/dG9rZW4~cGFydA== Bearer ***REDACTED***+c2VjcmV0/dG9rZW4~cGFydA== ``` `tokenPattern` is the Bearer rule in `RedactSensitiveData`, and through it in `ScrubUpstreamText`. Those functions scrub connection errors, `last_error`/health detail and tailed log lines. It is also used in `LogSafeRequestPath`. The value-shaped detector that runs afterwards does not recognise the leftover tail, so it ends up in the output. The log sanitizer's `bearer_token` pattern (`internal/logs/sanitizer.go`) and the audit redactor already use the RFC 6750 class. ## Trigger An upstream that rejects a request and echoes the credential in its error body, for example `upstream rejected Bearer dGhpcy1pcy1h+c2VjcmV0/dG9rZW4~cGFydA== (401)`. After `ScrubUpstreamText`, everything from the first `+` on is still visible in `last_error`. ## Fix Use `[a-zA-Z0-9\-_\.~+/]+=*` for the token value. ## Test `TestRedactSensitiveData/redacts_the_whole_of_a_base64_bearer_token` expects exactly `upstream rejected Bearer ***REDACTED*** (401)`. ## Commands run - `CGO_ENABLED=0 go test -count=1 ./internal/oauth`: ok - `CGO_ENABLED=0 go test -count=1 ./internal/httpapi ./internal/transport ./internal/logs`: ok - With `logging.go` reverted, the new subtest fails (`actual: "upstream rejected Bearer ***REDACTED***+c2VjcmV0/dG9rZW4~cGFydA== (401)"`). - `gofmt -l internal/oauth`: no output
1 parent 7f815fa commit 4fd87dc

2 files changed

Lines changed: 11 additions & 2 deletions

File tree

‎internal/oauth/logging.go‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -137,8 +137,10 @@ var sensitiveParams = []string{
137137
"credential",
138138
}
139139

140-
// tokenPattern matches Bearer tokens and other sensitive token patterns.
141-
var tokenPattern = regexp.MustCompile(`(?i)(bearer\s+)[a-zA-Z0-9\-_\.]+`)
140+
// tokenPattern matches Bearer tokens and other sensitive token patterns. The
141+
// value class is RFC 6750's b64token, so a standard-base64 token is not cut at
142+
// its first '+' or '/'.
143+
var tokenPattern = regexp.MustCompile(`(?i)(bearer\s+)[a-zA-Z0-9\-_\.~+/]+=*`)
142144

143145
// secretPattern matches common secret patterns.
144146
var secretPattern = regexp.MustCompile(`(?i)(secret|password|token|key)["']?\s*[:=]\s*["']?[a-zA-Z0-9\-_\.]+`)

‎internal/oauth/logging_test.go‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,13 @@ func TestRedactSensitiveData(t *testing.T) {
1717
assert.NotContains(t, result, "eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9")
1818
})
1919

20+
t.Run("redacts the whole of a base64 bearer token", func(t *testing.T) {
21+
// RFC 6750 b64token: '+', '/', '~' and trailing '=' are all legal.
22+
input := "upstream rejected Bearer dGhpcy1pcy1h+c2VjcmV0/dG9rZW4~cGFydA== (401)"
23+
result := RedactSensitiveData(input)
24+
assert.Equal(t, "upstream rejected Bearer ***REDACTED*** (401)", result)
25+
})
26+
2027
t.Run("redacts secrets in key=value format", func(t *testing.T) {
2128
input := "client_secret=my-super-secret-value"
2229
result := RedactSensitiveData(input)

0 commit comments

Comments
 (0)