Copilot commented on code in PR #864:
URL: https://github.com/apache/ranger/pull/864#discussion_r2870185249


##########
authz-embedded/src/main/java/org/apache/ranger/authz/embedded/RangerAuthzAuditHandler.java:
##########
@@ -44,7 +43,7 @@ public void processResult(RangerAccessResult result) {
             AuthzAuditEvent auditEvent = getAuthzEvents(result);
 
             if (auditEvent != null) {
-                auditEvent.setAgentId(plugin.getAppId());
+                auditEvent.setAgentId(appType);
 

Review Comment:
   `processResult()` sets `auditEvent.setAgentId(appType)` unconditionally. If 
a caller constructs this handler with a blank/whitespace `appType`, audits will 
end up with an empty `agent` field (and `RangerDefaultAuditHandler` defaults 
won't overwrite because the value is non-null). Consider normalizing `appType` 
in the constructor (e.g., trim-to-null) and/or only setting agentId when 
`appType` is non-blank so defaults can apply.



##########
authz-embedded/src/main/java/org/apache/ranger/authz/embedded/RangerAuthzAuditHandler.java:
##########
@@ -66,4 +65,8 @@ public void processResults(Collection<RangerAccessResult> 
results) {
     public void close() {
         auditEvents.forEach(super::logAuthzAudit);
     }
+
+    protected Collection<AuthzAuditEvent> getAuditEvents() {
+        return auditEvents;
+    }

Review Comment:
   `getAuditEvents()` returns the mutable internal `auditEvents` collection. 
Since this is the same collection later iterated in `close()`, any 
subclass/caller that modifies it can accidentally drop or duplicate audit 
logging. Returning an unmodifiable view or a defensive copy would avoid 
exposing internal state.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to