Skip to content

Store: reserve identity audit dimension (AuditEvent.actor + bootstrap/mismatch kinds) (#219) - #253

Merged
George-RD merged 1 commit into
mainfrom
dev-219
Aug 21, 2026
Merged

Store: reserve identity audit dimension (AuditEvent.actor + bootstrap/mismatch kinds) (#219)#253
George-RD merged 1 commit into
mainfrom
dev-219

Conversation

@George-RD

@George-RD George-RD commented Aug 21, 2026

Copy link
Copy Markdown
Owner

What

Fills the reserved identity audit dimension in the kernel Store (spec #208 D-007). Moves owner facts out of audit reason strings into the typed AuditEvent.actor dimension (delivered by #201) and adds the two reserved identity audit kinds, following #201's hash pre-image discipline exactly — new rows only, historical hashes never rewritten.

Why

Per spec #208 D-007: let a later tenancy/identity audit query filter by actor without reopening the Store interface. Owner facts previously lived in opaque reason strings (owner={} identity={}); the bootstrap-binding and owner-config-mismatch paths emitted no audit at all. This closes both gaps while keeping resolution read-only and identifiers hash-only.

Changes

  • AuditDescriptor gains an actor: Option<PrincipalId> field + with_actor builder; with_audited_effect now routes through append_audit_conn_with_actor so the actor is folded into the hashed audit pre-image.
  • event_bus replay agreement list gains "actor" (missed by Retype ApprovalRecord.approved_by and add AuditEvent.actor #201) so the field is cross-validated meta_jsonevent_json. Safe for legacy rows (meta lacks key → Null; event_json serializes actor: null).
  • identity.bound carries actor = owner principal; the owner fact is removed from the reason string (reason keeps only the bound-identity reference).
  • identity.bootstrapped (new kind): owner-creation routes through with_audited_effect (Immediate tx), emitting the kind with actor = new owner principal, atomic with the owner insert. Unique-constraint race recovery preserved.
  • identity.owner_config_mismatch (new kind): config mismatch commits a durable rejection audit (actor = stored owner) via append_audit_with_actor, then fails closed with NotOwner.
  • Resolution paths (get_identity, principal_exists, resolve_identity_by_identifier_hash) stay read-only; identifiers stay hash-only.

Verification

./scripts/check.sh green (exit 0). New tests in identity_tests.rs: actor on all three kinds, idempotent re-bootstrap emits no second row, mismatch still returns NotOwner, resolution writes no audit rows, and no raw identifier is persisted across event_json/meta_json/identity_identifiers.

Implementation notes (5-line summary)

  • Deviations: 1 — owner_config_mismatch uses append_audit_with_actor, not with_audited_effect (audit-only path per the audited_effect.rs module contract that reserves the combinator for effect writes).
  • Most likely to be revisited: the mismatch primitive choice, if a reviewer reads the ticket text literally.
  • Edge cases: bootstrap tx upgraded Deferred→Immediate (race recovery preserved); event_bus agreement list was missing "actor".
  • New audit kinds: identity.bootstrapped, identity.owner_config_mismatch; identity.bound now carries typed actor.
  • Read first next: crates/openspine-kernel/src/store/identity.rs and audited_effect.rs.

Closes #219


Summary by cubic

Reserves the identity audit dimension by moving owner facts from audit reasons into the typed AuditEvent.actor and adding two identity audit kinds. This enables filtering by actor without changing the Store interface and keeps identifiers hash-only.

  • Old: identity.bound encoded owner in reason. New: sets actor = owner principal and keeps reason to the bound identity; actor is folded into the audit hash pre-image. Side effect: event_bus replay now validates "actor" across meta_json and event_json, remaining safe for legacy rows (null when absent).

  • Old: owner bootstrap wrote no audit. New: emits identity.bootstrapped with actor = new owner principal, atomic with the owner insert via with_audited_effect; idempotent re-bootstrap writes no second row.

  • Old: config-owner mismatch wrote no audit. New: emits identity.owner_config_mismatch with actor = stored owner, then rejects with NotOwner.

  • AuditDescriptor adds actor and with_actor; with_audited_effect routes through append_audit_conn_with_actor so actor is included in the hashed pre-image. Historical rows are unchanged.

  • Resolution paths remain read-only; identifier handling remains hash-only, with no raw IDs persisted.

  • Migration: Consumers of audit logs should read actor instead of parsing owner from reason. Handle actor: null for legacy events. No data migration required.

Written for commit 4fc3d3c. Summary will update on new commits.

Review in cubic

…s) (#219)

Move owner facts out of audit reason strings into the typed
AuditEvent.actor dimension (spec #208 D-007), and add the two reserved
identity audit kinds, following #201's hash pre-image discipline
(new rows only; historical hashes untouched).

- AuditDescriptor gains an `actor` field + `with_actor` builder;
  with_audited_effect now routes through append_audit_conn_with_actor
  so the actor is folded into the audit pre-image.
- event_bus replay agreement list gains "actor" (missed by #201) so the
  field is cross-validated meta_json<->event_json. Safe for legacy rows.
- identity.bound carries actor = owner principal; owner fact removed from
  the reason string (reason keeps only the bound-identity reference).
- identity.bootstrapped: owner-creation now routes through
  with_audited_effect (Immediate tx), emitting the kind with actor = the
  new owner principal, atomic with the owner insert. Unique-constraint
  race recovery preserved.
- identity.owner_config_mismatch: config mismatch commits a durable
  rejection audit (actor = stored owner) via append_audit_with_actor,
  then fails closed with NotOwner.
- Resolution paths stay read-only; identifiers stay hash-only.
@coderabbitai

coderabbitai Bot commented Aug 21, 2026

Copy link
Copy Markdown

Warning

Review limit reached

@George-RD, you've reached your PR review limit, so we couldn't start this review.

Next review available in: 43 minutes

Limit details: You’ve used the included review currently available.

You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository.

How can I continue?

Wait for the limit to reset, then comment @coderabbitai review or push new commits to the PR.

An organization admin can change what happens after included review limits in Billing.

How do review limits work?

CodeRabbit enforces per-developer PR review limits within each organization.

For paid Pro and Pro+ reviews, CodeRabbit uses a developer's included PR review attempts over the past 7 days to set the current hourly allowance. At typical activity levels, the full plan allowance applies. Higher sustained activity can lower the allowance until earlier attempts leave the 7-day window.

Please refer docs for additional details.

Review details
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 1952f0b5-f67b-4e50-bae3-cad67fe6a9c4

📥 Commits

Reviewing files that changed from the base of the PR and between 65d0f0e and 4fc3d3c.

📒 Files selected for processing (4)
  • crates/openspine-kernel/src/store/audited_effect.rs
  • crates/openspine-kernel/src/store/event_bus.rs
  • crates/openspine-kernel/src/store/identity.rs
  • crates/openspine-kernel/src/store/identity_tests.rs

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@George-RD
George-RD merged commit ac30339 into main Aug 21, 2026
2 of 3 checks passed
@George-RD
George-RD deleted the dev-219 branch August 21, 2026 07:25

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 4fc3d3c02b

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment on lines +72 to +74
pub fn with_actor(mut self, actor: PrincipalId) -> Self {
self.actor = Some(actor);
self

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Forward descriptor actors through begin_effect

When an AuditDescriptor built with .with_actor(...) is passed to Store::begin_effect—the other existing consumer of this descriptor—effect_settlement.rs still calls the actor-less append_audit_conn, so the actor is silently persisted as null. This loses attribution for any owner-authored fenced effect despite the new descriptor contract; that path should pass audit.actor to append_audit_conn_with_actor as well.

Useful? React with 👍 / 👎.

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.

Store: reserve identity audit dimension (AuditEvent.actor + bootstrap/mismatch kinds)

1 participant