OAK-12331: document the audit SPI - #3058
Draft
dulvac wants to merge 6 commits into
Draft
Conversation
Adds two pages under the security section: a consumer guide covering the event model, the two producer paths, the commit metadata keys, the trust model and the listener contract; and a design document covering the pipeline internals, OSGi and embedded wiring, and the threading rules the implementation relies on. Both are linked from the security overview page and the site menu. The pages describe the SPI added by this issue, so they are written against that implementation rather than as a proposal.
joerghoh
reviewed
Jul 30, 2026
| There are two delivery paths: | ||
|
|
||
| - **Commit-attached.** Oak-internal capture sites (e.g. `UserManagerImpl`) | ||
| call `AuditEvents.record(root, event)`. Events land in a per-session |
Contributor
There was a problem hiding this comment.
What about operations which do not require an explicit session.save()?
Member
Author
There was a problem hiding this comment.
Writing "the surrounding Root.commit()" made this sound like i meant session.save(). I'll reword to say "the commit that follows, explicit or implicit".
The one case with no drain is a write that never goes through MutableRoot at all. e.g. oak-upgrade calling NodeStore.merge directly. That's the Migration commits section.
Contributor
|
For the implementation I am missing some basic metrics in the implementation, for example:
|
Member
Author
|
Good catch, there are no metrics at all right now. A slow listener runs synchronously on the commit thread, so it directly costs commit latency, and there's currently no way to see that from outside. I'll wire a Both will go in the implementation PR with a monitoring section here. If you have other metric counters we should add, let me know |
Follow-up to the review on PR apache#3058: - Define "capture site" on first use, instead of leaving the term to context. - Say explicitly that the drain hangs off the commit rather than off Session.save(), and name the operations that commit implicitly. - Scope the segment-store dispatch caveat to the segment store, and state that it does not apply to the document or composite stores. - Add a clustering section: events are node-local, what that means for a cluster-wide listener, and where the node id comes from. - Replace the hand-rolled "check for three payload keys" advice with AuditEvents.hasCommitMetadata(event), and publish the key names as constants on AuditEvent. - Add a monitoring section covering the per-domain event meter, the per-listener timer and failure meter, and the dropped-event meter. - Move Design rules to the top of the Implementation chapter and restate its back-references so each rule stands alone. The SPI helper, the key constants, and the metrics are documented here but implemented in the companion PR apache#3059.
The previous commit fixed audit-design.md only. Two of the review points apply to audit.md as well: - Define "capture site" there too, before its first use. - Note that the drain does not depend on an explicit Session.save(), and name the operations that commit on their own. Also reworded the commit-attached bullet in audit-design.md, where the "explicit or implicit" aside had buried the verb, and gave AuditEvents a javadoc link now that the page refers to it by name.
The docs described names that the implementation does not use. Corrected against the code on the implementation branch: - The attestation check is AuditEvent.isCommitAttested(event), not AuditEvents.hasCommitMetadata(event). - The reserved payload keys are oak.commit.sessionId / .userId / .timestamp, not commit.*. - The pipeline owner class is AuditPipeline, not AuditConfigurationImpl. - Domain and type are the AuditDomain / AuditType value types rather than bare strings, so SecurityAuditDomain exposes DOMAIN rather than NAME and UserAuditTypes holds AuditType constants. Updated the three code samples that still implemented the string-based interface. Also documented how the metrics resolve their StatisticsProvider, and why the event meter counts once per domain per commit rather than once per delivery.
The design doc named AuditDomain and AuditType without saying why they exist. The rationale is from the review on PR apache#3059: constraining the value at construction keeps a domain usable as a JCR node name, so a listener that persists events into the repository can build a path from it without escaping. Records the actual rules (non-blank, JcrNameParser, no colon, no whitespace), why the colon is rejected rather than escaped, and why neither type is an enum. Also fixes a bullet list in audit.md that an earlier edit had split in two, orphaning Timestamp and Payload below a paragraph.
dulvac
added a commit
to dulvac/jackrabbit-oak
that referenced
this pull request
Aug 5, 2026
Requested in review on PR apache#3058: there was no way to see the pipeline from outside, and a slow listener runs on the commit thread, so it costs commit latency with nothing to point at. AuditMonitor wraps a StatisticsProvider and records: - security.audit.events;domain=<domain> — events dispatched per domain - security.audit.events.dropped;domain=<domain> — events discarded at the per-session buffer cap - security.audit.listener.duration;listener=<class> — time in onEvents - security.audit.listener.failures;listener=<class> — listener throws The provider is looked up on the whiteboard rather than injected as a DS reference, so OSGi and embedded callers share the one path through initialize(). Deployments without a provider get AuditMonitor.NOOP. Two counting decisions worth knowing about. An event is counted once per domain, not once per delivery, so N listeners on a domain do not multiply the rate. And it is counted only when a listener actually consumed it: a listener that unregisters between capture and drain leaves a domain in the grouped map that nothing consumed, and counting that would overstate the rate. Both are covered by AuditMonitorWiringTest. Recording sits inside the existing per-listener Throwable barriers, so a metrics failure cannot break a dispatch. A listener that throws is still timed: it burned commit-thread time before it threw.
Member
Author
|
@joerghoh thanks again for the review. I addressed your feedback in the latest changes. please have a look when you can. |
…he docs AuditEvents is now AuditDispatch, and AuditEventImpl moved to the unexported spi.audit.impl package. Both pages referred to the old names. Also records why AuditEventImpl sits outside the exported package, since that is the kind of thing the next reader will otherwise undo.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Documentation for the audit SPI added under OAK-12331.
I'm opening this ahead of the implementation on purpose. The whole change is
large, and it's much easier to review in two chunks than in one, so the docs
come first and the code is ready to follow straight after. Reviewing this
first also means any disagreement about the SPI's shape or its contracts
surfaces here, in prose, rather than after the implementation is written.
Two new pages under
oak-doc/src/site/markdown/security/:audit.md, the consumer guide: event model, the two producer paths(commit-attached and fire-and-forget), the
commit.*metadata keys,configuring and probing the pipeline, emitting events, implementing a
listener, and the trust model.
audit-design.md, the design document: pipeline internals, SPI layoutacross
oak-core-spi/oak-security-spi/oak-core, OSGi and embeddedwiring, and the threading and ordering rules.
Both are linked from
security/overview.mdand nested under Security insite.xml.Docs only, so please don't merge ahead of the implementation, or the site
will describe an SPI that isn't in the release.
Notable design choices
toggle. Deliberate: draining unconditionally stops a mid-flight toggle flip
from stranding staged events for a later commit to misattribute.
called
Root.commit(), beforemergereturns, because the event buffer isa
ThreadLocal. It does not depend on every store dispatching throughChangeDispatcher:MemoryNodeStorenotifies its observers directly fromsetRoot.cold-standby instance, and a store configured through
SegmentNodeStoreFactorywithoutdispatchChangesset.behind. Documented as a gap in the trail, since it matters for anyone
building a compliance record.
mvn site -Pdocbuilds clean and RAT passes. The embedded wiring snippet hasbeen compiled and run.