Keep one audit mode through metadata and emission (C1 I3)

Carry the captured enhanced mode into event construction without rereading the property. Preserve public delegating overloads and keep every multi parent/member on the same mode. Cover deterministic ACL off-to-on and multi mode transitions with subsequent genuine v2 emission.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Author: arkmish

zookeeper-docs/src/main/resources/markdown/zookeeperAuditLogs.md

@@ -210,8 +210,12 @@ Consumers should recognize v2 per record using `schema_version=2`, tolerate
 unknown additive fields, and accept omitted optional fields. Do not apply v2
 unescaping to legacy records. Prepare consumers for mixed legacy/v2 output before
 enabling the property on servers, then roll it out gradually. Roll back emission
-by removing the enhanced property or setting it to `false` on restart. The
-enhanced gate is checked when emitting events; the base audit enablement retains
+by removing the enhanced property or setting it to `false` on restart. Each
+audited request captures the enhanced mode before metadata extraction, and uses
+that same decision for user/ACL sanitization, schema construction and every
+parent/member record of a multi. An in-flight request keeps its captured mode if
+the property changes; a subsequent request captures the new value. Direct
+provider logging captures its mode per event. The base audit enablement retains
 its startup behavior. No wire, persistence, authentication, ACL, quota or payload
 limit changes are required.
 

zookeeper-server/src/main/java/org/apache/zookeeper/audit/AuditHelper.java

@@ -383,7 +383,7 @@ private static void log(Request request, ProcessTxnResult rc, String path, Strin
         }
         ZKAuditProvider.log(getUsers(request, enhanced), operation, path, metadata.acl, metadata.createMode,
                 request.cnxn.getSessionIdHex(), request.cnxn.getHostAddress(), result,
-                metadata.dataLength, error, outcome, request.cxid, zxid, index);
+                metadata.dataLength, error, outcome, request.cxid, zxid, index, enhanced);
     }
 
     private static void auditError(int type, Exception e) {

zookeeper-server/src/main/java/org/apache/zookeeper/audit/ZKAuditProvider.java

@@ -90,8 +90,19 @@ public static void log(String user, String operation, String znode, String acl,
         if (!isAuditEnabled()) {
             return;
         }
+        log(user, operation, znode, acl, createMode, session, ip, result,
+                dataLength, errorCode, outcome, cxid, zxid, multiIndex, isEnhancedAuditEnabled());
+    }
+
+    static void log(String user, String operation, String znode, String acl,
+                    String createMode, String session, String ip, Result result,
+                    Integer dataLength, Integer errorCode, Outcome outcome,
+                    Integer cxid, Long zxid, Integer multiIndex, boolean enhanced) {
+        if (!isAuditEnabled()) {
+            return;
+        }
         logAuditEvent(createLogEvent(user, operation, znode, acl, createMode, session, ip, result,
-                dataLength, errorCode, outcome, cxid, zxid, multiIndex));
+                dataLength, errorCode, outcome, cxid, zxid, multiIndex, enhanced));
     }
 
     /**
@@ -101,7 +112,7 @@ static AuditEvent createLogEvent(String user, String operation, Result result) {
         AuditEvent event = new AuditEvent(result);
         event.addEntry(FieldName.USER, user);
         event.addEntry(FieldName.OPERATION, operation);
-        addMetadata(event, null, null, null, null, null, null);
+        addMetadata(event, null, null, null, null, null, null, isEnhancedAuditEnabled());
         return event;
     }
 
@@ -118,6 +129,14 @@ static AuditEvent createLogEvent(String user, String operation, String znode, St
                                      String createMode, String session, String ip, Result result,
                                      Integer dataLength, Integer errorCode, Outcome outcome,
                                      Integer cxid, Long zxid, Integer multiIndex) {
+        return createLogEvent(user, operation, znode, acl, createMode, session, ip, result,
+                dataLength, errorCode, outcome, cxid, zxid, multiIndex, isEnhancedAuditEnabled());
+    }
+
+    private static AuditEvent createLogEvent(String user, String operation, String znode, String acl,
+                                             String createMode, String session, String ip, Result result,
+                                             Integer dataLength, Integer errorCode, Outcome outcome,
+                                             Integer cxid, Long zxid, Integer multiIndex, boolean enhanced) {
         AuditEvent event = new AuditEvent(result);
         event.addEntry(FieldName.SESSION, session);
         event.addEntry(FieldName.USER, user);
@@ -126,13 +145,13 @@ static AuditEvent createLogEvent(String user, String operation, String znode, St
         event.addEntry(FieldName.ZNODE, znode);
         event.addEntry(FieldName.ZNODE_TYPE, createMode);
         event.addEntry(FieldName.ACL, acl);
-        addMetadata(event, dataLength, errorCode, outcome, cxid, zxid, multiIndex);
+        addMetadata(event, dataLength, errorCode, outcome, cxid, zxid, multiIndex, enhanced);
         return event;
     }
 
     private static void addMetadata(AuditEvent event, Integer dataLength, Integer errorCode, Outcome outcome,
-                                    Integer cxid, Long zxid, Integer multiIndex) {
-        if (isEnhancedAuditEnabled()) {
+                                    Integer cxid, Long zxid, Integer multiIndex, boolean enhanced) {
+        if (enhanced) {
             event.addEntry(FieldName.SCHEMA_VERSION, AuditConstants.SCHEMA_VERSION);
             event.addEntry(FieldName.DATA_LENGTH, valueOf(dataLength));
             event.addEntry(FieldName.ERROR_CODE, valueOf(errorCode));

zookeeper-server/src/test/java/org/apache/zookeeper/audit/AuditHelperTest.java

@@ -66,6 +66,7 @@
 import org.apache.zookeeper.server.ServerCnxn;
 import org.apache.zookeeper.server.ServerMetrics;
 import org.apache.zookeeper.server.auth.AuthenticationProvider;
+import org.apache.zookeeper.server.auth.DigestAuthenticationProvider;
 import org.apache.zookeeper.server.auth.ProviderRegistry;
 import org.apache.zookeeper.txn.CheckVersionTxn;
 import org.apache.zookeeper.txn.CloseSessionTxn;
@@ -74,6 +75,7 @@
 import org.apache.zookeeper.txn.DeleteTxn;
 import org.apache.zookeeper.txn.ErrorTxn;
 import org.apache.zookeeper.txn.MultiTxn;
+import org.apache.zookeeper.txn.SetACLTxn;
 import org.apache.zookeeper.txn.SetDataTxn;
 import org.apache.zookeeper.txn.Txn;
 import org.apache.zookeeper.txn.TxnHeader;
@@ -416,6 +418,115 @@ public void testEnhancedUsersRedactUnknownAndMalformedIdentities() throws Except
         assertFalse(log.contains("synthetic"));
     }
 
+    @Test
+    public void testAclModeSnapshotSurvivesOffToOnInterleaving() throws Exception {
+        Request create = request(OpCode.create, createRecord("/mode-acl", new byte[0], CreateMode.PERSISTENT));
+        assertEquals(0, apply(create, OpCode.create, createTxn("/mode-acl", new byte[0], false)).err);
+        String digest = DigestAuthenticationProvider.generateDigest("alice:synthetic-password");
+        List<ACL> acls = Collections.singletonList(new ACL(ZooDefs.Perms.ALL, new Id("digest", digest)));
+        SetACLRequest record = new SetACLRequest("/mode-acl", acls, -1);
+        AtomicInteger transitions = new AtomicInteger();
+        Request switching = new Request(cnxn, SESSION, 41, OpCode.setACL, ByteBuffer.wrap(serialize(record)),
+                Collections.singletonList(new Id("ip", "127.0.0.1"))) {
+            @Override
+            public String getUsers() {
+                System.setProperty(ENHANCED_ENABLE, "true");
+                transitions.incrementAndGet();
+                return super.getUsers();
+            }
+        };
+        ProcessTxnResult changed = apply(switching, OpCode.setACL, new SetACLTxn("/mode-acl", acls, 1));
+        assertEquals(0, changed.err);
+        System.setProperty(ENHANCED_ENABLE, "false");
+        AuditHelper.addAuditLog(switching, changed);
+        Map<String, String> legacy = fields(capture.read(1).get(0));
+        assertEquals(1, transitions.get());
+        assertEquals("true", System.getProperty(ENHANCED_ENABLE));
+        assertNull("An in-flight legacy record must not be relabeled v2", legacy.get("schema_version"));
+        assertEquals("digest:" + digest + ":cdrwa", legacy.get("acl"));
+        assertEquals("success", legacy.get("result"));
+
+        Request enhanced = request(OpCode.setACL, record);
+        ProcessTxnResult updated = apply(enhanced, OpCode.setACL, new SetACLTxn("/mode-acl", acls, 2));
+        assertEquals(0, updated.err);
+        AuditHelper.addAuditLog(enhanced, updated);
+        String log = capture.read(1).get(0);
+        assertWrite(fields(log), "setAcl", "/mode-acl", null, "committed", "0");
+        assertEquals("digest:alice:cdrwa", fields(log).get("acl"));
+        assertFalse(log.contains(digest));
+        assertFalse(log.contains("synthetic-password"));
+    }
+
+    @Test
+    public void testSuccessfulMultiKeepsOneModeAcrossMembers() throws Exception {
+        Request request = request(OpCode.multi, new MultiOperationRecord(Arrays.asList(
+                Op.create("/mode-multi", new byte[1], ZooDefs.Ids.OPEN_ACL_UNSAFE, CreateMode.PERSISTENT),
+                Op.setData("/mode-multi", new byte[2], -1))));
+        ProcessTxnResult result = apply(request, OpCode.multi, new MultiTxn(Arrays.asList(
+                txn(OpCode.create, createTxn("/mode-multi", new byte[1], false)),
+                txn(OpCode.setData, new SetDataTxn("/mode-multi", new byte[2], 1)))));
+        assertEquals(0, result.err);
+        System.setProperty(ENHANCED_ENABLE, "false");
+        AtomicInteger events = new AtomicInteger();
+        AuditLogger delegate = new Slf4jAuditLogger();
+        Object previousLogger = replaceProviderField("auditLogger", (AuditLogger) event -> {
+            delegate.logAuditEvent(event);
+            if (events.incrementAndGet() == 1) {
+                System.setProperty(ENHANCED_ENABLE, "true");
+            }
+        });
+        try {
+            AuditHelper.addAuditLog(request, result);
+            List<String> logs = capture.read(2);
+            assertEquals(2, events.get());
+            for (String log : logs) {
+                assertNull("A multi must keep its captured legacy mode", fields(log).get("schema_version"));
+                assertEquals("success", fields(log).get("result"));
+            }
+
+            Request following = request(OpCode.setData, new SetDataRequest("/mode-multi", new byte[3], -1));
+            ProcessTxnResult updated = apply(following, OpCode.setData, new SetDataTxn("/mode-multi", new byte[3], 2));
+            assertEquals(0, updated.err);
+            AuditHelper.addAuditLog(following, updated);
+            assertWrite(fields(capture.read(1).get(0)), "setData", "/mode-multi", "3", "committed", "0");
+        } finally {
+            replaceProviderField("auditLogger", previousLogger);
+        }
+    }
+
+    @Test
+    public void testFailedMultiKeepsParentModeForRolledBackMembers() throws Exception {
+        Request request = request(OpCode.multi, new MultiOperationRecord(Arrays.asList(
+                Op.create("/mode-rolled", new byte[1], ZooDefs.Ids.OPEN_ACL_UNSAFE, CreateMode.PERSISTENT),
+                Op.check("/missing", -1),
+                Op.setData("/mode-rolled", new byte[2], -1))));
+        ProcessTxnResult result = apply(request, OpCode.multi, new MultiTxn(Arrays.asList(
+                txn(OpCode.create, createTxn("/mode-rolled", new byte[1], false)),
+                txn(OpCode.error, new ErrorTxn(Code.NONODE.intValue())),
+                txn(OpCode.error, new ErrorTxn(Code.RUNTIMEINCONSISTENCY.intValue())))));
+        assertEquals(-101, result.err);
+        assertNull(tree.getNode("/mode-rolled"));
+        AtomicInteger events = new AtomicInteger();
+        AuditLogger delegate = new Slf4jAuditLogger();
+        Object previousLogger = replaceProviderField("auditLogger", (AuditLogger) event -> {
+            delegate.logAuditEvent(event);
+            if (events.incrementAndGet() == 1) {
+                System.setProperty(ENHANCED_ENABLE, "false");
+            }
+        });
+        try {
+            AuditHelper.addAuditLog(request, result);
+            List<String> logs = capture.read(3);
+            assertEquals(3, events.get());
+            assertWrite(fields(logs.get(0)), "multiOperation", null, null, "failed", "-101");
+            assertWrite(fields(logs.get(1)), "create", "/mode-rolled", "1", "rolled_back", "0");
+            assertWrite(fields(logs.get(2)), "setData", "/mode-rolled", "2", "rolled_back", "-2");
+            assertEquals("2", fields(logs.get(2)).get("multi_index"));
+        } finally {
+            replaceProviderField("auditLogger", previousLogger);
+        }
+    }
+
     @Test
     public void testAuditDisabledSkipsRequestsAndProvider() throws Exception {
         Object previous = replaceProviderField("auditEnabled", false);