Skip to content

debug(api-server): add V(4) logging to pre-auth stream interceptor #1464

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.

Sign up for GitHub

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

Closed
markturansky wants to merge 1 commit into from
Closed

debug(api-server): add V(4) logging to pre-auth stream interceptor #1464

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 14 additions & 0 deletions components/ambient-api-server/pkg/middleware/bearer_token_grpc.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import (
"strings"

"github.com/golang-jwt/jwt/v4"
"github.com/golang/glog"
"github.com/openshift-online/rh-trex-ai/pkg/auth"
"google.golang.org/grpc"
"google.golang.org/grpc/metadata"
Expand Down Expand Up @@ -52,19 +53,32 @@ func bearerTokenGRPCStreamInterceptor(expectedToken, serviceAccountUsername stri
if authHeader := md.Get("authorization"); len(authHeader) > 0 {
if token, err := extractBearerToken(authHeader[0]); err == nil {
if subtle.ConstantTimeCompare([]byte(token), []byte(expectedToken)) == 1 {
glog.V(4).Infof("[pre-auth] stream %s: static token match, setting CallerTypeService", info.FullMethod)
return handler(srv, &serviceCallerStream{ServerStream: ss, ctx: withCallerType(ss.Context(), CallerTypeService)})
}
if username := usernameFromJWT(token); username != "" {
ctx := auth.SetUsernameContext(ss.Context(), username)
if serviceAccountUsername != "" && username == serviceAccountUsername {
glog.V(4).Infof("[pre-auth] stream %s: OIDC username %q matches service account, setting CallerTypeService", info.FullMethod, username)
ctx = withCallerType(ctx, CallerTypeService)
} else {
glog.V(4).Infof("[pre-auth] stream %s: OIDC username %q (service account %q, match=%v)", info.FullMethod, username, serviceAccountUsername, username == serviceAccountUsername)
}
Comment on lines +62 to 66
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot Apr 24, 2026

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

Avoid logging raw usernames/service-account identifiers in pre-auth path

These messages log user identifiers directly. Even at V(4), this creates avoidable privacy/compliance risk in retained logs. Keep only match/state booleans (or hash/redact identifiers).

Suggested change 
-							glog.V(4).Infof("[pre-auth] stream %s: OIDC username %q matches service account, setting CallerTypeService", info.FullMethod, username)
+							glog.V(4).Infof("[pre-auth] stream %s: OIDC username matches configured service account, setting CallerTypeService", info.FullMethod)
 							ctx = withCallerType(ctx, CallerTypeService)
 						} else {
-							glog.V(4).Infof("[pre-auth] stream %s: OIDC username %q (service account %q, match=%v)", info.FullMethod, username, serviceAccountUsername, username == serviceAccountUsername)
+							glog.V(4).Infof("[pre-auth] stream %s: OIDC username present (service account configured=%t, match=%v)", info.FullMethod, serviceAccountUsername != "", username == serviceAccountUsername)
 						}

As per coding guidelines, **/*: Flag bugs, security vulnerabilities, logic errors, data loss risks, and meaningful refactoring opportunities.

🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@components/ambient-api-server/pkg/middleware/bearer_token_grpc.go` around
lines 62 - 66, The current pre-auth logs in the bearer_token_grpc path emit raw
identifiers (username and serviceAccountUsername) via glog.V(4).Infof; replace
those prints with non-identifying information only — e.g., log the match result
and method name and/or a redacted/hashed form of the identifier instead of the
plaintext username. Update the two logging sites that reference info.FullMethod,
username, and serviceAccountUsername (the glog.V(4).Infof calls surrounding the
withCallerType(ctx, CallerTypeService) branch) to remove raw identifiers and
emit either username==serviceAccountUsername (boolean) or a deterministic
hash/redaction token while preserving the existing CallerTypeService behavior.
Ensure any redaction/hashing uses a stable, non-reversible method and does not
change the control flow in withCallerType or the branch that sets
CallerTypeService.

return handler(srv, &serviceCallerStream{ServerStream: ss, ctx: ctx})
} else {
glog.V(4).Infof("[pre-auth] stream %s: usernameFromJWT returned empty, token length=%d", info.FullMethod, len(token))
}
} else {
glog.V(4).Infof("[pre-auth] stream %s: extractBearerToken error: %v", info.FullMethod, err)
}
} else {
glog.V(4).Infof("[pre-auth] stream %s: no authorization header in metadata", info.FullMethod)
}
} else {
glog.V(4).Infof("[pre-auth] stream %s: no incoming metadata", info.FullMethod)
}

glog.V(4).Infof("[pre-auth] stream %s: falling through without CallerType", info.FullMethod)
return handler(srv, ss)
}
}
Expand Down
Loading