dulvac commented on code in PR #3059: URL: https://github.com/apache/jackrabbit-oak/pull/3059#discussion_r3711357232
########## oak-core/src/main/java/org/apache/jackrabbit/oak/security/audit/CommitMetadataDecorator.java: ########## @@ -0,0 +1,217 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.apache.jackrabbit.oak.security.audit; + +import java.util.ArrayList; +import java.util.Collections; +import java.util.HashMap; +import java.util.List; +import java.util.Map; +import java.util.Objects; +import java.util.concurrent.atomic.AtomicBoolean; + +import org.apache.jackrabbit.oak.spi.audit.AuditEvent; +import org.apache.jackrabbit.oak.spi.commit.CommitInfo; +import org.jetbrains.annotations.NotNull; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +/** + * Enforces the {@code commit.*} trust contract at both dispatch + * boundaries: {@link #decorate} stamps commit-attached payloads with + * commit metadata (sessionId, userId, timestamp) at drain time, and + * {@link #stripReservedCommitKeys} removes caller-supplied values for the + * same three keys from fire-and-forget payloads before delivery. + * <p> + * Both operations return NEW {@link AuditEvent} instances that wrap the + * originals; the input events are not mutated. The wrapper's payload map + * is unmodifiable. + * + * <h3>Security invariant</h3> + * Both halves enforce the same property: listeners can treat the presence + * of {@link #KEY_SESSION_ID}, {@link #KEY_USER_ID}, or {@link #KEY_TIMESTAMP} + * in a dispatched payload as Oak-attested — see the normative trust contract + * on {@link org.apache.jackrabbit.oak.spi.audit.AuditEvent#getPayload()}. + * {@link #decorate} <strong>unconditionally overwrites</strong> the three + * keys with the values from the {@link CommitInfo} captured for the + * surrounding commit; {@link #stripReservedCommitKeys} removes + * caller-supplied values for the same keys on the fire-and-forget path. + * Weakening either half — {@code putIfAbsent} / {@code computeIfAbsent} / + * conditional {@code put} in the decorator, or skipping the strip at + * dispatch — is a regression in the trust model. + * + * <h3>Payload null-value contract</h3> + * The decorator trusts the no-null-keys/no-null-values contract documented + * on {@link org.apache.jackrabbit.oak.spi.audit.AuditEventListener#onEvents}. + * Buggy event implementations that violate it may leak null values to + * listeners — runtime validation is the event author's responsibility, + * not the decorator's. Adding per-entry null checks here would impose + * hot-path cost for what is an SPI-contract violation. + */ +final class CommitMetadataDecorator { + + private static final Logger log = LoggerFactory.getLogger(CommitMetadataDecorator.class); + + static final String KEY_SESSION_ID = "commit.sessionId"; + static final String KEY_USER_ID = "commit.userId"; + static final String KEY_TIMESTAMP = "commit.timestamp"; Review Comment: makes sense. I'll rename to `oak.commit.*` unless you'd prefer a different prefix. -- 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]
