Skip to content

[Audit Sink][PART1] Introduce Audit Sink Trait - #1189

Open
HatemMn wants to merge 6 commits into
developfrom
feat/audit-sink-refactor
Open

HatemMn wants to merge 6 commits into
developfrom
feat/audit-sink-refactor

Conversation

@HatemMn

@HatemMn HatemMn commented Sep 11, 2026

Copy link
Copy Markdown
Contributor

reminder of the order

ci/audit-jsonl-compat
└─ feat/audit-sink-refactor (4 commits)
└─ feat/audit-sink-generalize (1 commit)
└─ feat/audit-postgres-backend (2 commits)
└─ feat/audit-postgres-cli (3 commits)


Overview

This PR is just a refactoring/a split for the 2090-line audit file store into three modules before touching anything real:

file_sink.rs (main file), writer.rs (the writer loop), store.rs (the handle + tests), recovery.rs (recovery).

Also pulled event construction into one place in access, split the fat middlewares/audit.rs into mod.rs + client_ip.rs + extensions.rs, and added the new AuditSink trait in interfaces (unused so far).

No behavior change anywhere in this PR.

You might notice that two audit traits are living separatly: one of them is the legacy file trait, it will be removed in the next PR

@HatemMn
HatemMn deployed to xks-remote-approval September 11, 2026 15:42 — with GitHub Actions Active
@HatemMn
HatemMn marked this pull request as ready for review September 11, 2026 15:43
@HatemMn
HatemMn added this pull request to stack #1128 September 11, 2026 15:47
@HatemMn
HatemMn removed this pull request from stack #1128 September 11, 2026 15:49
@HatemMn
HatemMn added this pull request to stack #1196 September 15, 2026 08:52
@HatemMn HatemMn self-assigned this Sep 15, 2026
@HatemMn
HatemMn deployed to xks-remote-approval September 15, 2026 12:31 — with GitHub Actions Active
@HatemMn
HatemMn deployed to xks-remote-approval September 15, 2026 13:48 — with GitHub Actions Active
@HatemMn HatemMn changed the title [Audit Sink][PART1] Feat/audit sink refactor [Audit Sink][PART1] Introduce Audit Sink Trait Sep 15, 2026
@Manuthor
Manuthor removed this pull request from stack #1196 September 15, 2026 16:28
@HatemMn HatemMn assigned Manuthor and unassigned HatemMn Sep 15, 2026
@HatemMn
HatemMn requested a review from Manuthor September 15, 2026 17:49
Base automatically changed from ci/audit-jsonl-compat to develop September 16, 2026 09:00
@HatemMn
HatemMn added this pull request to stack #1200 September 16, 2026 12:33
Introduce RequestAuditContext/OperationAuditContext + AuditEventDraft::build() so the
single-operation path and the per-BatchItem fan-out share one constructor instead
of two positional-argument helpers (make_success_draft/make_failure_draft) plus a
third direct struct literal in the batch path -- three places a newly added field
could silently go missing from one of them.

Also adds:
- AuditResult::from_canonical_str(), the inverse of as_canonical_str(), needed to
  rebuild an event from a persisted column instead of serde's own enum encoding.
- audit_now(): current UTC time truncated to microsecond resolution, so a
  PostgreSQL TIMESTAMPTZ round-trip reproduces byte-identical canonical bytes.

Preserves the current AuditEvent.details field and the pre-details-field
hash-compatibility fixture, which the reference implementation this is adapted
from (feat/audit_pgSQL) predates and does not have.
Mechanical split of middlewares/audit.rs into middlewares/audit/{mod,client_ip,extensions}.rs:
- extensions.rs: request-extension marker types (KmipOperationName, KmipObjectUid,
  KmipAlgorithm, BatchItemAuditContext, KmipBatchOperations), re-exported unchanged
  from mod.rs so external call sites (middlewares/mod.rs, routes/kmip/audit.rs) need
  no changes.
- client_ip.rs: extract_operation / extract_client_ip and their tests.
- mod.rs: AuditMiddleware/AuditService (Transform/Service impl), unchanged behavior.

No backend changes. Ran the log-reference.md generator (--non-interactive) to update
the 2 call-site paths that moved; left 3 unrelated pre-existing [REMOVED]-flagged
entries (src/routes/kmip/handlers.rs) untouched -- out of scope for this commit.
Adds AuditSink (interfaces::stores::audit_sink), a generic trait for the durable
destination of finalised audit events: resume() -> ChainHead, write_event(),
write_failure_is_fatal(), final_sync(). No implementors yet -- FileSink lands in
the next commit (extracted from the current AuditFileStore), PgAuditSink after
that.

Recovery policy is explicitly per-backend, not part of this contract: resume()'s
doc does not mandate a single global policy (e.g. 'always fail-fast on tail
corruption'). A backend whose writes can be torn mid-write may recover a
trustworthy prefix; a backend whose writes are atomic can reasonably fail closed
on any tail corruption. Each implementor documents its own choice.

Adds cosmian_kms_access as a dependency of cosmian_kms_interfaces (no existing
reverse dependency -- verified no cycle) to reference AuditEvent in the trait
signature.
Split file_store.rs (2090 lines) into 3 modules, no behavior change:
- file_sink.rs: recovery/classification logic, private sync AuditSink mock trait, lock/open/seal helpers
- writer.rs: writer_loop, write_draft_to_chain, eviction sentinel
- store.rs: AuditFileStore handle + WriterMsg + all tests

Prepares for generalizing over the new public cosmian_kms_interfaces::AuditSink trait in the next commit.
@HatemMn
HatemMn force-pushed the feat/audit-sink-refactor branch from e9c0a4b to c92a46c Compare September 16, 2026 14:18
@HatemMn
HatemMn deployed to xks-remote-approval September 16, 2026 14:18 — with GitHub Actions Active
/// Position of the audit hash chain: the id to assign to the next event, and the
/// `row_hash` of the last durably persisted one.
#[derive(Debug, Clone, Copy)]
pub struct ChainHead {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Suggested change
pub struct ChainHead {
#[derive(Debug, Clone, Copy, PartialEq, Eq, Default)]
pub struct ChainHead {

Derive PartialEq, Eq, and Default on ChainHead so instances can be directly compared (e.g. head == ChainHead::EMPTY) and defaulted in consumers and unit tests.

//! is given, and reports where the chain left off so the writer can resume it. Backends
//! are interchangeable at the trait boundary: a chain started on one backend can be
//! verified after export from another, because both encode the same [`AuditEvent`] and
//! the same canonical hash (see `cosmian_kms_access::audit::canonical_bytes`).

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Suggested change
//! the same canonical hash (see `cosmian_kms_access::audit::canonical_bytes`).
//! the same canonical hash (see `cosmian_kms_access::audit::compute_row_hash`).

canonical_bytes is private (pub(crate)) inside cosmian_kms_access::audit::hash and not re-exported. Public callers and documentation should reference compute_row_hash or cosmian_kms_access::audit.

});
};
match check_row(anchor_line) {
RowCheck::Verified(event) => match event.id.checked_add(1) {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Suggested change
RowCheck::Verified(event) => match event.id.checked_add(1) {
match check_row(anchor_line) {
RowCheck::Verified(event) => {
if !verify_chain_link(&event, previous_event) {
return Ok(TailOutcome::SealAndRoll {
reason: SealReason::ChainBroken,
claimed_last_id: Some(event.id),
failure_offset: last_start,
});
}
match event.id.checked_add(1) {
Some(next_id) => Ok(TailOutcome::TruncateContinue {
keep_len: last_start,
next_id,
prev_hash: event.row_hash,
bytes_discarded: last_end - last_start,
discard_offset: last_start,
}),
None => Ok(TailOutcome::SealAndRoll {
reason: SealReason::IdOverflow,
claimed_last_id: Some(event.id),
failure_offset: last_start,
}),
}
}
RowCheck::HashMismatch(_) | RowCheck::Unparsable => Ok(TailOutcome::SealAndRoll {

When recovering from a torn trailing write, verify verify_chain_link(&event, previous_event) on the fallback anchor event before accepting it as the truncation boundary. If the fallback anchor row itself has a broken hash link, classify_tail should trigger SealAndRoll rather than resuming over a corrupted chain.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants