diff --git a/crates/credentials-core/src/audit.rs b/crates/credentials-core/src/audit.rs index b1dc323..72954bf 100644 --- a/crates/credentials-core/src/audit.rs +++ b/crates/credentials-core/src/audit.rs @@ -139,6 +139,39 @@ pub enum AuthEventKind { GithubAppPermissionsChanged, } +/// The consumer-asserted, unverified source of a reported authentication failure. +/// +/// This is separate from vault-observed `AuthEventKind` and `detail`: the vault records +/// what the reporter claims about its own path, without vouching for that claim. The +/// closed set ensures consumer input can never become durable plaintext. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub enum ReporterSource { + Direct, + RelayStatusField, + RelayMessageParse, + Unrecognised, +} + +impl ReporterSource { + pub const fn as_str(self) -> &'static str { + match self { + Self::Direct => "direct", + Self::RelayStatusField => "relay_status_field", + Self::RelayMessageParse => "relay_message_parse", + Self::Unrecognised => "unrecognised", + } + } + + pub fn from_wire(value: &str) -> Self { + match value { + "direct" => Self::Direct, + "relay_status_field" => Self::RelayStatusField, + "relay_message_parse" => Self::RelayMessageParse, + _ => Self::Unrecognised, + } + } +} + impl AuthEventKind { /// The stable storage string for this authentication diagnostic kind. pub fn as_str(self) -> &'static str { @@ -503,7 +536,7 @@ mod tests { #[cfg(test)] mod vocabulary_documentation_tests { - use super::{AlarmReason, AuditOp, AuthEventKind}; + use super::{AlarmReason, AuditOp, AuthEventKind, ReporterSource}; const RUNBOOK: &str = include_str!(concat!( env!("CARGO_MANIFEST_DIR"), @@ -667,4 +700,38 @@ mod vocabulary_documentation_tests { value(AuthEventKind::GithubAppPermissionsChanged), ); } + + #[test] + fn reporter_source_rejects_unrecognised_wire_values() { + assert_eq!( + ReporterSource::from_wire(&"a".repeat(40)), + ReporterSource::Unrecognised + ); + } + + #[test] + fn reporter_source_values_are_documented() { + let section = documented_subsection( + "#### `auth_events.reporter_source`", + "**Table:** `auth_events`", + ); + + fn value(source: ReporterSource) -> &'static str { + match source { + ReporterSource::Direct => ReporterSource::Direct.as_str(), + ReporterSource::RelayStatusField => ReporterSource::RelayStatusField.as_str(), + ReporterSource::RelayMessageParse => ReporterSource::RelayMessageParse.as_str(), + ReporterSource::Unrecognised => ReporterSource::Unrecognised.as_str(), + } + } + + for source in [ + ReporterSource::Direct, + ReporterSource::RelayStatusField, + ReporterSource::RelayMessageParse, + ReporterSource::Unrecognised, + ] { + assert_documented(section, "auth_events.reporter_source", value(source)); + } + } } diff --git a/crates/credentials-core/src/engine.rs b/crates/credentials-core/src/engine.rs index fea2bcc..0ebf535 100644 --- a/crates/credentials-core/src/engine.rs +++ b/crates/credentials-core/src/engine.rs @@ -249,6 +249,7 @@ impl RefreshEngine { kind: AuthEventKind::StaleNonrefreshableLatch.as_str(), provider_status: None, detail: None, + reporter_source: None, }), )?; // If a concurrent write moved the version, return the replacement rather @@ -435,6 +436,7 @@ impl RefreshEngine { kind: AuthEventKind::RefreshFailed.as_str(), provider_status: None, detail: Some(e.variant_name()), + reporter_source: None, }), )?; Err(EngineError::RefreshFailed(e)) @@ -459,6 +461,7 @@ impl RefreshEngine { kind: AuthEventKind::RefreshFailed.as_str(), provider_status: other.provider_status(), detail: Some(other.variant_name()), + reporter_source: None, }, Some(record.record_version), ); diff --git a/crates/credentials-core/src/engine_tests.rs b/crates/credentials-core/src/engine_tests.rs index db6706e..1bb7271 100644 --- a/crates/credentials-core/src/engine_tests.rs +++ b/crates/credentials-core/src/engine_tests.rs @@ -693,6 +693,7 @@ async fn report_stale_then_invalid_grant_latches_needs_reauth() { kind: "consumer_report_stale", provider_status: Some(401), detail: None, + reporter_source: None, }, ) .expect("report marks the current token stale"); @@ -735,6 +736,7 @@ async fn stale_pending_clears_in_the_refresh_commit() { kind: "consumer_report_stale", provider_status: Some(401), detail: None, + reporter_source: None, }, ) .expect("report marks the current token stale"); diff --git a/crates/credentials-core/src/lib.rs b/crates/credentials-core/src/lib.rs index f2d9014..fda52e3 100644 --- a/crates/credentials-core/src/lib.rs +++ b/crates/credentials-core/src/lib.rs @@ -48,7 +48,7 @@ pub use admin_auth::{ ADMIN_NONCE_LEN, ADMIN_TAG_LEN, VAULT_ID_LEN, }; pub use admin_ops::{AdminAuditOp, AdminOpBody, StoreMode, ADMIN_OP_SCHEMA_V1}; -pub use audit::{AlarmReason, AuditEntry, AuditOp, AuditRecord, AuthEventKind}; +pub use audit::{AlarmReason, AuditEntry, AuditOp, AuditRecord, AuthEventKind, ReporterSource}; pub use contract::{keychain_service_for, vault_id_for, MODULE_ID, STORAGE_NAMESPACE}; pub use credential_id::{ default_refresh_adapter, parse_credential_id, AuthMethod, ParsedCredentialId, diff --git a/crates/credentials-core/src/store.rs b/crates/credentials-core/src/store.rs index 81dd7e7..5f2fb92 100644 --- a/crates/credentials-core/src/store.rs +++ b/crates/credentials-core/src/store.rs @@ -43,7 +43,9 @@ use rusqlite::OptionalExtension; use sha2::{Digest, Sha256}; use zeroize::Zeroizing; -use crate::audit::{self, AlarmReason, AuditCtx, AuditEntry, AuditOp, AuditRecord, AuthEventKind}; +use crate::audit::{ + self, AlarmReason, AuditCtx, AuditEntry, AuditOp, AuditRecord, AuthEventKind, ReporterSource, +}; use crate::envelope::{self, EnvelopeError, RecordBinding}; use crate::key::{KeyId, MasterKey}; pub use crate::record::RecordState; @@ -232,6 +234,10 @@ const MIGRATIONS: &[Migration] = &[ version: 7, statements: "ALTER TABLE credentials ADD COLUMN last_github_app_permissions TEXT;", }, + Migration { + version: 8, + statements: "ALTER TABLE auth_events ADD COLUMN reporter_source TEXT;", + }, ]; /// The newest store migration THIS BINARY knows how to apply. @@ -2037,6 +2043,7 @@ impl EncryptedStore { kind: AuthEventKind::GithubAppPermissionsChanged.as_str(), provider_status: None, detail: Some(&detail), + reporter_source: None, }, Some(record_version), true, @@ -2928,26 +2935,34 @@ pub fn read_auth_events_read_only( /// are two readers (leased and lease-free), and duplicating the pair would mean two /// places that have to be changed together with nothing to catch a miss. const AUTH_EVENTS_SELECT: &str = - "SELECT ts_ms, credential_id, kind, provider_status, detail, record_version, applied, \ + "SELECT ts_ms, credential_id, kind, provider_status, detail, reporter_source, record_version, applied, \ principal_kind, principal_id \ FROM auth_events ORDER BY seq DESC LIMIT ?1"; // A read-only CLI can run before the daemon has restarted to apply migration 4. Keep // its existing diagnostics readable by projecting absent principal fields as NULL. const AUTH_EVENTS_SELECT_PRE_PRINCIPAL: &str = - "SELECT ts_ms, credential_id, kind, provider_status, detail, record_version, applied, \ + "SELECT ts_ms, credential_id, kind, provider_status, detail, NULL AS reporter_source, record_version, applied, \ NULL AS principal_kind, NULL AS principal_id \ FROM auth_events ORDER BY seq DESC LIMIT ?1"; +const AUTH_EVENTS_SELECT_PRE_REPORTER_SOURCE: &str = + "SELECT ts_ms, credential_id, kind, provider_status, detail, NULL AS reporter_source, record_version, applied, \ + principal_kind, principal_id \ + FROM auth_events ORDER BY seq DESC LIMIT ?1"; + fn auth_events_select(conn: &rusqlite::Connection) -> rusqlite::Result<&'static str> { let mut stmt = conn.prepare("PRAGMA table_info(auth_events)")?; let columns = stmt .query_map([], |row| row.get::<_, String>(1))? .collect::>>()?; - if columns.iter().any(|column| column == "principal_kind") - && columns.iter().any(|column| column == "principal_id") - { + let has_principal = columns.iter().any(|column| column == "principal_kind") + && columns.iter().any(|column| column == "principal_id"); + let has_reporter_source = columns.iter().any(|column| column == "reporter_source"); + if has_principal && has_reporter_source { Ok(AUTH_EVENTS_SELECT) + } else if has_principal { + Ok(AUTH_EVENTS_SELECT_PRE_REPORTER_SOURCE) } else { Ok(AUTH_EVENTS_SELECT_PRE_PRINCIPAL) } @@ -2962,10 +2977,11 @@ fn auth_event_from_row(r: &rusqlite::Row<'_>) -> rusqlite::Result { kind: r.get(2)?, provider_status: r.get::<_, Option>(3)?.map(|s| s as u16), detail: r.get(4)?, - record_version: r.get::<_, Option>(5)?.map(|v| v as u64), - applied: r.get::<_, i64>(6)? != 0, - principal_kind: r.get(7)?, - principal_id: r.get(8)?, + reporter_source: r.get(5)?, + record_version: r.get::<_, Option>(6)?.map(|v| v as u64), + applied: r.get::<_, i64>(7)? != 0, + principal_kind: r.get(8)?, + principal_id: r.get(9)?, }) } @@ -3003,6 +3019,8 @@ pub struct AuthEvent { pub kind: String, pub provider_status: Option, pub detail: Option, + /// Consumer-asserted, unverified; from `ReporterSource::as_str`, never raw consumer input. + pub reporter_source: Option, pub record_version: Option, /// Whether this observation actually changed the credential. False for a report /// against a superseded version, and for events that authorise no change. @@ -3096,6 +3114,8 @@ pub struct AuthObservation<'a> { pub provider_status: Option, /// A typed variant or locally rendered safe metadata. Never response text. pub detail: Option<&'a str>, + /// Consumer-asserted, unverified; raw consumer input is unrepresentable here. + pub reporter_source: Option, } /// Append one `auth_events` row. Diagnostics only: not MAC-chained, prunable, and @@ -3113,14 +3133,15 @@ pub(crate) fn append_auth_event_tx( ) -> rusqlite::Result<()> { tx.execute( "INSERT INTO auth_events \ - (ts_ms, credential_id, kind, provider_status, detail, record_version, applied) \ - VALUES (?1, ?2, ?3, ?4, ?5, ?6, ?7)", + (ts_ms, credential_id, kind, provider_status, detail, reporter_source, record_version, applied) \ + VALUES (?1, ?2, ?3, ?4, ?5, ?6, ?7, ?8)", rusqlite::params![ now_ms(), credential_id, obs.kind, obs.provider_status.map(|s| s as i64), obs.detail, + obs.reporter_source.map(ReporterSource::as_str), record_version.map(|v| v as i64), applied as i64, ], @@ -3783,6 +3804,7 @@ mod tests { kind: "consumer_report", provider_status: Some(401), detail: None, + reporter_source: None, }, Some(1), ) @@ -4771,6 +4793,7 @@ mod tests { kind: "consumer_report_stale", provider_status: Some(401), detail: None, + reporter_source: None, }, ) .expect("stale report is accepted"); @@ -4818,6 +4841,7 @@ mod tests { kind: "consumer_report", provider_status: Some(401), detail: None, + reporter_source: None, }; // Stale report against v1 while the store holds v2. @@ -5060,6 +5084,7 @@ mod tests { kind: "consumer_report", provider_status: Some(401), detail: None, + reporter_source: None, }; // First report at the served version: a real transition. @@ -5133,6 +5158,7 @@ mod tests { kind: "refresh_failed", provider_status: Some(503), detail: Some("status"), + reporter_source: None, }, Some(1), ) @@ -5148,6 +5174,7 @@ mod tests { kind: "consumer_report", provider_status: Some(401), detail: None, + reporter_source: None, }, Some(1), ) @@ -5273,7 +5300,7 @@ mod tests { fn the_newest_migration_version_is_pinned_because_the_manifest_declares_it() { assert_eq!( newest_migration_version(), - 7, + 8, "the newest migration changed. This value is DECLARED in the module manifest \ as store_schema_version, so a supervisor comparing declared-against-actual \ sees it. Update the literal, and note the manifest consequence." diff --git a/crates/credentials-module/src/bin/credentials_cli.rs b/crates/credentials-module/src/bin/credentials_cli.rs index 58921f1..6187147 100644 --- a/crates/credentials-module/src/bin/credentials_cli.rs +++ b/crates/credentials-module/src/bin/credentials_cli.rs @@ -2955,8 +2955,15 @@ fn cmd_events(global: &GlobalArgs, args: &[String]) -> Result<(), CliError> { .record_version .map(|v| format!("v{v}")) .unwrap_or_else(|| "-".into()); + // The consumer-asserted source rides at the end and only when present, so + // legacy NULL rows render exactly as before this column existed. + let source = e + .reporter_source + .as_deref() + .map(|s| format!(" src={s}")) + .unwrap_or_default(); println!( - "{when} {:34} {:16} {principal:24} {what:22} {version:6} applied={}", + "{when} {:34} {:16} {principal:24} {what:22} {version:6} applied={}{source}", e.credential_id, e.kind, if e.applied { "yes" } else { "no" } diff --git a/crates/credentials-module/src/main.rs b/crates/credentials-module/src/main.rs index bfec236..5f46d15 100644 --- a/crates/credentials-module/src/main.rs +++ b/crates/credentials-module/src/main.rs @@ -525,6 +525,7 @@ fn record_reconciliation_reasons( kind: AuthEventKind::ReconcileNeedsReauth.as_str(), provider_status: None, detail: Some(reason.as_str()), + reporter_source: None, }, None, ); @@ -4599,6 +4600,7 @@ mod tests { handle: handle.raw.clone(), provider_status: 401, record_version: 1, + reporter_source: None, }, ) .await @@ -4726,6 +4728,7 @@ mod tests { handle: raw.raw.clone(), provider_status: 401, record_version: 1, + reporter_source: None, }, ) .await @@ -5172,16 +5175,19 @@ mod tests { .1 .state }; - let params = |status: u16, version: u64| read_surface::ReportAuthFailureParams { - handle: handle.clone(), - provider_status: status, - record_version: version, + let params = |status: u16, version: u64, reporter_source: Option<&str>| { + read_surface::ReportAuthFailureParams { + handle: handle.clone(), + provider_status: status, + record_version: version, + reporter_source: reporter_source.map(str::to_owned), + } }; // A NON-AUTH status must not invalidate: a provider 500 is a hiccup, not a dead // credential. surface - .report_auth_failure(7, ¶ms(500, 1)) + .report_auth_failure(7, ¶ms(500, 1, None)) .await .expect("a non-auth status is accepted"); assert_eq!( @@ -5200,7 +5206,7 @@ mod tests { ) .expect("bump the record version"); surface - .report_auth_failure(7, ¶ms(401, 1)) + .report_auth_failure(7, ¶ms(401, 1, Some("relay_message_parse"))) .await .expect("a stale report is accepted, not errored"); assert_eq!( @@ -5209,11 +5215,21 @@ mod tests { "a 401 for a version the vault has moved past must NOT invalidate: that \ credential was already repaired" ); + let events = store.recent_auth_events(10).expect("stale report event"); + assert_eq!(events[0].kind, "consumer_report_latch"); + assert_eq!( + events[0].reporter_source.as_deref(), + Some("relay_message_parse") + ); + assert!( + !events[0].applied, + "a state no-op still records a diagnostic observation" + ); // THE ACCEPTED ARM. Without it, an implementation that ignored every report // satisfies both assertions above. surface - .report_auth_failure(7, ¶ms(401, 2)) + .report_auth_failure(7, ¶ms(401, 2, Some(&"a".repeat(40)))) .await .expect("a current-version 401 is accepted"); assert_eq!( @@ -5237,6 +5253,21 @@ mod tests { ); let events = store.recent_auth_events(10).expect("events"); assert_eq!(events[0].kind, "consumer_report_latch"); + assert_eq!(events[0].reporter_source.as_deref(), Some("unrecognised")); + let raw = "a".repeat(40); + assert!( + events.iter().all(|event| { + event.credential_id != raw + && event.kind != raw + && event.detail.as_deref() != Some(raw.as_str()) + && event.reporter_source.as_deref() != Some(raw.as_str()) + && event.principal_kind.as_deref() != Some(raw.as_str()) + && event.principal_id.as_deref() != Some(raw.as_str()) + }), + "the raw reporter source must never appear in any string column of \ + auth_events -- not merely mapped out of reporter_source itself, but not \ + displaced into detail or the principal fields either" + ); assert!( events[0].applied, "the current static report must be recorded as applied" @@ -5251,6 +5282,7 @@ mod tests { handle: "ckh_not_a_handle".to_string(), provider_status: 401, record_version: 1, + reporter_source: None, }, ) .await; @@ -5302,6 +5334,7 @@ mod tests { handle: handle.raw, provider_status: 401, record_version: 1, + reporter_source: None, }, ) .await @@ -5360,6 +5393,7 @@ mod tests { handle: handle.raw.clone(), provider_status: 401, record_version: 1, + reporter_source: None, }, ) .await diff --git a/crates/credentials-module/src/read_surface.rs b/crates/credentials-module/src/read_surface.rs index 8800200..632fd99 100644 --- a/crates/credentials-module/src/read_surface.rs +++ b/crates/credentials-module/src/read_surface.rs @@ -15,7 +15,7 @@ //! for a route-bound reserved principal with a literal-prefix read grant. //! - `credential.get_many { items: [...] }` → capped at [`limiter::GET_MANY_MAX`]. //! - `credential.status { handle? }` → non-secret health, never bytes. -//! - `credential.report_auth_failure { handle, provider_status, record_version }` → +//! - `credential.report_auth_failure { handle, provider_status, record_version, reporter_source? }` → //! marks the token STALE on a refreshable credential so the next get REFRESHES it, //! and latches `needs_reauth` only for a non-refreshable one. A refresh that then //! returns `invalid_grant` latches through the path that already existed. Measured @@ -41,7 +41,9 @@ use base64::Engine; use serde::{Deserialize, Serialize}; use tokio::sync::Mutex; -use credentials_core::audit::{AlarmReason, AuditCtx, AuditOp, AuditRecord, AuthEventKind}; +use credentials_core::audit::{ + AlarmReason, AuditCtx, AuditOp, AuditRecord, AuthEventKind, ReporterSource, +}; use credentials_core::credential_id::{default_refresh_adapter, parse_credential_id}; use credentials_core::engine::{EngineError, RefreshEngine}; use credentials_core::health::VaultHealth; @@ -242,6 +244,11 @@ pub struct ReportAuthFailureParams { /// no-op instead of falsely killing the fresh token. A consumer that omits it is /// rejected (`invalid_params`) rather than silently invalidating whatever is current. pub record_version: u64, + /// Optional consumer-asserted observation-path label from the closed [`ReporterSource`] + /// vocabulary. Unknown labels are recorded as `unrecognised`, never stored raw; older + /// consumers may omit this field. + #[serde(default)] + pub reporter_source: Option, } /// A successful `get` result. `payload` is opaque to the consumer. @@ -1118,6 +1125,10 @@ impl ReadSurface { }, provider_status: Some(params.provider_status), detail: None, + reporter_source: params + .reporter_source + .as_deref() + .map(ReporterSource::from_wire), }; if refreshable { self.engine @@ -1472,6 +1483,17 @@ fn map_engine_error(e: &EngineError) -> ReadError { mod error_class_tests { use super::*; + #[test] + fn report_auth_failure_params_default_missing_reporter_source() { + let params: ReportAuthFailureParams = serde_json::from_value(serde_json::json!({ + "handle": "ckh_example", + "provider_status": 401, + "record_version": 1 + })) + .expect("legacy report payload remains valid"); + assert_eq!(params.reporter_source, None); + } + /// Golden conformance: this producer's serde wire strings for `ErrorClass` match /// the pinned contract set exactly (order-independent, no extras, no misses). If a /// contract change ever alters the set, this fails loudly instead of drifting. diff --git a/crates/credentials-module/tests/cli_admin.rs b/crates/credentials-module/tests/cli_admin.rs index 7d77066..5133cd1 100644 --- a/crates/credentials-module/tests/cli_admin.rs +++ b/crates/credentials-module/tests/cli_admin.rs @@ -2159,6 +2159,7 @@ fn events_discloses_that_the_retention_cap_discarded_older_rows() { kind: "consumer_report", provider_status: Some(401), detail: None, + reporter_source: None, }, Some(1), ) @@ -2172,6 +2173,7 @@ fn events_discloses_that_the_retention_cap_discarded_older_rows() { kind: "consumer_report", provider_status: Some(401), detail: None, + reporter_source: None, }, Some(1), ) diff --git a/docs/cortexkit-credentials-contract.md b/docs/cortexkit-credentials-contract.md index 1b9e055..728162a 100644 --- a/docs/cortexkit-credentials-contract.md +++ b/docs/cortexkit-credentials-contract.md @@ -133,7 +133,7 @@ credential.get { handle, min_ttl_ms?, force_refresh? } credential.get_many { items: [{ handle, ... }] } (CAPPED — see §6) credential.status { handle? } → { result: { ready, last_error_code?, lease_held } } (non-secret health, never bytes) -credential.report_auth_failure { handle, provider_status, record_version } +credential.report_auth_failure { handle, provider_status, record_version, reporter_source? } → { result: { accepted: true } } (version-CAS feedback — §7) ``` @@ -253,9 +253,10 @@ v1 mechanisms, all buildable now: ## 7. Revocation propagation (HIGH) The vault must not serve a dead token until `expires_at_ms`: -- **`credential.report_auth_failure { handle, provider_status, record_version }`** +- **`credential.report_auth_failure { handle, provider_status, record_version, reporter_source? }`** (read-surface, rate-limited): a consumer that gets a provider 401/403 reports the - exact version it used. The vault marks the record `needs_reauth` and clears any + exact version it used. `reporter_source` optionally names the consumer's observation + path; it is unverified and unknown labels are stored as `unrecognised`. The vault marks the record `needs_reauth` and clears any dangling refresh intent only when that version still matches. If refresh or replace already advanced the record, the stale report is an accepted silent no-op. This CAS prevents a delayed 401 for version N from killing healthy version N+1. Consumers must diff --git a/docs/operator-runbook.md b/docs/operator-runbook.md index 69293c0..8481554 100644 --- a/docs/operator-runbook.md +++ b/docs/operator-runbook.md @@ -588,9 +588,9 @@ it did not exist for the first incident. If `reactivate` is followed within minutes by another report at the new version, the credential is genuinely dead and `login --replace` is the repair. -### The three diagnostic string vocabularies +### The four diagnostic string vocabularies -The vault has three separate string vocabularies that are easy to confuse. They live in +The vault has four separate string vocabularies that are easy to confuse. They live in **different tables and columns**: #### `audit_log.op` @@ -673,7 +673,29 @@ value from a corrupt one. Other unknown kinds may still appear (a future retirement, a fixture from a test harness). Treat them as diagnostics, never as audit-log operations or alarm reasons -- -the three vocabularies are separate and a value from one is not a value from another. +the vocabularies are separate and a value from one is not a value from another. + +#### `auth_events.reporter_source` + +**Table:** `auth_events` + +**Column:** `reporter_source` (TEXT, nullable) + +This is a consumer-asserted, unverified claim about where the reported failure was +observed. It is structurally separate from `detail`, which records what the vault +observed. The vault accepts only this closed vocabulary; any other wire string becomes +`unrecognised`, and the original string is never stored: + +| Value | Meaning | +| --- | --- | +| `direct` | The consumer saw the provider status on a direct response. | +| `relay_status_field` | The consumer read a structured status field from a relay error event. | +| `relay_message_parse` | The consumer recovered the status by parsing relay message text. | +| `unrecognised` | The consumer supplied a source outside the vault's named vocabulary. | + +`NULL` means the report predates this column or the consumer omitted the optional field; +it is normal, not invalid data. This differs from `unrecognised`, which means the consumer +sent a value that the vault refused to store. ### Reading the chain directly