feat(approval): per-launch UUID session_id + scrub legacy bearer leaks

[oxoxDev]

Summary

• Remove session_id from public approval surface types (PendingApproval, ApprovalAuditEntry) and from the DomainEvent::ApprovalRequested bus payload.
• Construct session_id unconditionally as a per-launch UUID (session-<uuid>) inside bootstrap_core_runtime — no longer derived from the JSON-RPC bearer.
• Add a one-shot PRAGMA user_version=1 SQLite migration that scrubs legacy pending_approvals.session_id rows to a redacted sentinel. Column retained for downgrade safety.
• Add debug-build assertion + regression tests so the field/value never leak back through Debug, serialize, or audit channels.

Problem

The approval gate persisted the JSON-RPC bearer token verbatim as the session_id field on every persisted approval row, on every published ApprovalRequested domain event, and through any audit-log Debug surface. When the bearer was set as a stable value (the documented power-user / remote-core / docker-cloud path), this turned the on-disk approval.db SQLite file, any tracing / panic backtrace that touched the event, and any subscriber that Debug-printed a PendingApproval into a plaintext bearer-credential leak.

The pre-existing log-line <redacted> masking at src/core/jsonrpc.rs only suppressed the console surface — the credential still flowed into SQLite + the event bus + the gate's in-memory state.

Solution

Commit 1 — refactor(approval): drop session_id from public approval types

Remove the session_id: String field from PendingApproval (src/openhuman/approval/types.rs) and ApprovalAuditEntry. Remove session_id from DomainEvent::ApprovalRequested (src/core/event_bus/events.rs). Cascade compile-error fixes through callers; the field was never read by any frontend (verified via grep on app/src/components/chat/ApprovalRequestCard.tsx + redux store consumers).

Commit 2 — feat(core/jsonrpc): use unconditional per-launch UUID for approval session_id

Replace the match crate::core::auth::get_rpc_token() branch at the gate-installation site with let session_id = format!("session-{}", uuid::Uuid::new_v4()). The bearer remains the authentication credential; session_id is now strictly a per-launch correlation token. The old <redacted> label machinery is removed (no longer applicable — the value is always a UUID and always safe to log).

Commit 3 — feat(approval/store): scrub legacy session_id rows via PRAGMA user_version migration

One-shot migration runs inside the schema-init path on first open. PRAGMA user_version gates the rewrite so the scrub runs exactly once per database even across many process launches. Existing rows are overwritten with the literal pre-migration-redacted (exported as PRE_MIGRATION_SESSION_ID). The column itself is retained so a downgrade does not lose the row entirely.

Commit 4 — test(approval): regression guards on session_id removal

ApprovalGate::new carries a #[cfg(debug_assertions)] debug_assert! that the supplied session_id starts with the session- prefix. Three regression tests guarantee that:

1. PendingApproval Debug + serde JSON serialize never carry a session_id field.
2. ApprovalAuditEntry Debug never carries a session_id field.
3. DomainEvent::ApprovalRequested Debug never carries a session_id field.

Any future refactor that re-introduces a raw-bearer wiring trips the debug-build assertion immediately; any refactor that re-introduces the field name (via serde rename, manual Debug impl, or struct re-addition) fails the regression tests.

Submission Checklist

• Tests added or updated (happy path + at least one failure / edge case) per Testing Strategy
• Diff coverage ≥ 80% — every new branch (UUID construction, SQLite migration, debug-assertion gate, regression guards) is exercised by added tests in approval::store::tests, approval::types::tests, approval::gate::tests, core::event_bus::events::tests.
• N/A: Coverage matrix not updated — defense-in-depth hardening of an existing surface; no new feature row to add.
• N/A: All affected feature IDs from the matrix are listed under Related — defense-in-depth hardening, no matrix rows touched.
• No new external network dependencies introduced (no network surface touched at all).
• N/A: Manual smoke checklist updated — release-cut surfaces unchanged; this is an internal credential-handling fix with no user-visible UI surface.
• N/A: Linked issue closed via Closes #NNN — tracked privately; cross-referenced from the security-advisory thread instead of a public issue.

Impact

• Runtime: desktop (Windows, macOS, Linux) + CLI + docker/cloud — all hosts that boot the approval gate go through the new UUID construction path.
• Persistence: first open after upgrade triggers a one-shot UPDATE pending_approvals SET session_id = 'pre-migration-redacted' WHERE session_id != 'pre-migration-redacted' followed by PRAGMA user_version = 1. Audit history for in-flight prompts loses per-launch grouping (rows reflect the redacted sentinel); decided/expired rows are unaffected operationally.
• Public API: PendingApproval, ApprovalAuditEntry, and DomainEvent::ApprovalRequested lose their session_id field. Frontend not affected (field never consumed by ApprovalRequestCard or the chat-runtime slice). Any third-party subscriber on the event bus that read session_id would not compile against this branch — #[non_exhaustive] already shielded DomainEvent from breaking-pattern-match changes, but a positional struct destructure would need updating.
• Security: closes the credential leak across SQLite + event-bus + Debug-print surfaces. Bearer no longer leaves the RPC_TOKEN OnceLock for any audit/correlation purpose.
• Downgrade: the session_id column is retained on disk; an older binary still reads the column shape, just sees the redacted sentinel for any pre-upgrade rows.

Related

• Closes: tracked via the private security-advisory thread.
• Follow-up PR(s)/TODOs: ApprovalGate::session_id() accessor (gate.rs:466) currently has zero non-test callers — candidate for removal once the security-advisory window closes.

Summary by CodeRabbit

• Refactor
  • Removed sensitive per-session identifiers from all public approval payloads and types; approval events now expose only routing/correlation fields.
  • Approval data structures marked non-exhaustive for forward compatibility.
• Bug Fixes / Migrations
  • Database migration scrubs legacy session identifiers, preserves audit integrity, and is idempotent.
  • Internal session keys are now generated per-launch and persisted for audit grouping.
• Tests
  • Added regression tests ensuring session identifiers are not exposed and migrations behave correctly.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: bbe42dba-df32-47e4-a9c8-440833a4c28b

📥 Commits

Reviewing files that changed from the base of the PR and between 5acfc58 and a995c86.

📒 Files selected for processing (2)

• src/core/event_bus/events.rs
• src/core/jsonrpc.rs

💤 Files with no reviewable changes (2)

• src/core/jsonrpc.rs
• src/core/event_bus/events.rs

---

📝 Walkthrough

Walkthrough

Removes session_id from public approval types and events, adds an idempotent migration that scrubs legacy DB session values to a sentinel, updates store API/tests to accept an explicit session argument, enforces internal session-<uuid> format in ApprovalGate, and mints per-boot session IDs at bootstrap.

Changes

Session ID Privacy Hardening

Layer / File(s)	Summary
Public API type model—remove session_id from PendingApproval and ApprovalAuditEntry src/openhuman/approval/types.rs	PendingApproval and ApprovalAuditEntry remove public session_id field, add #[non_exhaustive], and document that session_id is intentionally excluded from the public/serde shape. Regression tests verify session_id does not appear in Debug or JSON outputs.
Event payload simplification—remove session_id from DomainEvent::ApprovalRequested src/core/event_bus/events.rs, src/core/event_bus/events_tests.rs	DomainEvent::ApprovalRequested variant removes session_id field; doc comments clarify provenance is internal to ApprovalGate. A new test asserts Debug formatting does not contain session_id.
Persistence—schema migration and storage contract updates src/openhuman/approval/store.rs	Adds PRE_MIGRATION_SESSION_ID and migrate_session_id_scrub that overwrites legacy pending_approvals.session_id values and bumps PRAGMA user_version to make the scrub idempotent. insert_pending now requires explicit session_id: &str. Row mappers stop exposing session_id.
Persistence—store test updates for new insert_pending signature src/openhuman/approval/store.rs (tests)	Threads explicit session_id through store tests and adjusts sample helpers to stop embedding session_id into PendingApproval. Adds migration test verifying overwrite to sentinel and idempotency across reopen.
Gate integration—enforce session format and thread session_id to store src/openhuman/approval/gate.rs	ApprovalGate docs and debug-only assertion enforce session-<uuid> format. Gate no longer sets session_id on PendingApproval, instead passing &self.session_id into store::insert_pending. Gate removes session_id from published DomainEvent::ApprovalRequested.
Bootstrap—generate fresh per-boot session identifiers src/core/jsonrpc.rs, src/openhuman/agent/harness/tool_loop_tests.rs	bootstrap_core_runtime now always generates a fresh session-<uuid> and initializes ApprovalGate with it; logging updated. E2E test adjusted to use session-shaped gate name.

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~60 minutes

Possibly related PRs

• tinyhumansai/openhuman#2149: Related earlier change to approval gating flow that introduced session handling now being hardened and re-scoped.
• tinyhumansai/openhuman#2335: Overlaps approval audit/persistence modeling; that PR surfaces session_id in audit types which this change removes.
• tinyhumansai/openhuman#2709: Related bootstrap/session-id derivation changes affecting ApprovalGate initialization.

Suggested reviewers

• graycyrus
• senamakel

Poem

🐇 I hopped in at boot with a tidy new name,
session-UUID kept private, never bearing your claim.
SQLite hides the trace, legacy scrubbed away,
Public types stay calm — no secret shows today.
A rabbit's small scrub, tidy paths for play.

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title accurately summarizes the main change: introducing a per-launch UUID session_id and removing legacy bearer-based session ID leaks. It captures both primary aspects of the changeset.
Docstring Coverage	✅ Passed	Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

---

_{Comment @coderabbitai help to get the list of available commands and usage tips.}