Skip to content

auth: records carry an account identity; set-identity and import set it, replace keeps it #30

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
ualtinok merged 2 commits into from
Sep 3, 2026
Merged

auth: records carry an account identity; set-identity and import set it, replace keeps it #30

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
e6c1482
auth: records carry an account identity; set-identity and import set …
iceteaSA Sep 3, 2026
20c7920
identity: a supplied account_id that contradicts the token's own clai…
iceteaSA Sep 3, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
98 changes: 95 additions & 3 deletions crates/credentials-core/src/admin_ops.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ use serde::{Deserialize, Serialize};

use crate::audit::AuditRecord;
use crate::audit::{AuditCtx, AuditOp};
use crate::record::VaultRecord;
use crate::record::{RecordIdentity, VaultRecord};
use crate::store::{mint_handle, EncryptedStore, GrantOperation, StoreOpError};

/// The admin-op schema version. Bumped only on a breaking op-body change; the
Expand All @@ -38,6 +38,24 @@ pub enum AdminOpBody {
audit_op: AdminAuditOp,
mode: StoreMode,
},
/// Identity-aware replacement has its own op discriminator so a daemon that predates
/// sticky identity rejects it instead of silently treating it as a legacy replace.
#[serde(rename = "admin.store_with_identity_policy")]
StoreWithIdentityPolicy {
v: u32,
id: String,
record: Box<VaultRecord>,
audit_op: AdminAuditOp,
clear_identity: bool,
},
/// Update only the non-secret identity attached to an existing record. The store
/// re-seals the unchanged credential material and keeps its lifecycle state.
#[serde(rename = "admin.set_identity")]
SetIdentity {
v: u32,
id: String,
identity: RecordIdentity,
},
#[serde(rename = "admin.invalidate")]
Invalidate { v: u32, id: String },
/// Reversibly stop serving a credential because an operator intentionally retired
Expand Down Expand Up @@ -152,6 +170,26 @@ impl std::fmt::Debug for AdminOpBody {
.field("audit_op", audit_op)
.field("mode", mode)
.finish(),
AdminOpBody::StoreWithIdentityPolicy {
v,
id,
record,
audit_op,
clear_identity,
} => f
.debug_struct("StoreWithIdentityPolicy")
.field("v", v)
.field("id", id)
.field("record", record)
.field("audit_op", audit_op)
.field("clear_identity", clear_identity)
.finish(),
AdminOpBody::SetIdentity { v, id, identity } => f
.debug_struct("SetIdentity")
.field("v", v)
.field("id", id)
.field("identity", identity)
.finish(),
AdminOpBody::Invalidate { v, id } => f
.debug_struct("Invalidate")
.field("v", v)
Expand Down Expand Up @@ -234,6 +272,8 @@ impl AdminOpBody {
pub fn schema_version(&self) -> u32 {
match self {
AdminOpBody::Store { v, .. }
| AdminOpBody::StoreWithIdentityPolicy { v, .. }
| AdminOpBody::SetIdentity { v, .. }
| AdminOpBody::Invalidate { v, .. }
| AdminOpBody::Logout { v, .. }
| AdminOpBody::Reactivate { v, .. }
Expand All @@ -260,6 +300,8 @@ impl AdminOpBody {
credential_id: id, ..
}
| AdminOpBody::Store { id, .. }
| AdminOpBody::StoreWithIdentityPolicy { id, .. }
| AdminOpBody::SetIdentity { id, .. }
| AdminOpBody::Invalidate { id, .. }
| AdminOpBody::Logout { id, .. }
| AdminOpBody::Reactivate { id, .. }
Expand Down Expand Up @@ -365,6 +407,29 @@ pub fn apply(
}
Ok(serde_json::json!({ "stored": true }))
}
AdminOpBody::StoreWithIdentityPolicy {
id,
record,
audit_op,
clear_identity,
..
} => {
store.overwrite_unconditional_with_identity_policy_audited(
&id,
&record,
!clear_identity,

@cubic-dev-ai cubic-dev-ai Bot Sep 2, 2026

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: When clear_identity is true but the wire record contains identity fields, this call only disables preservation of the old identity; it does not clear the incoming identity. Clear or reject record.identity before calling the store so the operation guarantees that --clear-identity drops identity for every authenticated caller.

Prompt for AI agents 
Check if this issue is valid — if so, understand the root cause and fix it. At crates/credentials-core/src/admin_ops.rs, line 420:

<comment>When `clear_identity` is true but the wire record contains identity fields, this call only disables preservation of the old identity; it does not clear the incoming identity. Clear or reject `record.identity` before calling the store so the operation guarantees that `--clear-identity` drops identity for every authenticated caller.</comment>

<file context>
@@ -365,6 +407,29 @@ pub fn apply(
+            store.overwrite_unconditional_with_identity_policy_audited(
+                &id,
+                &record,
+                !clear_identity,
+                AuditCtx::route_admin(audit_op.to_audit_op(), actor),
+            )?;
</file context>

AuditCtx::route_admin(audit_op.to_audit_op(), actor),
)?;
Ok(serde_json::json!({ "stored": true }))
}
AdminOpBody::SetIdentity { id, identity, .. } => {
store.set_identity_audited(
&id,
identity,
AuditCtx::route_admin(AuditOp::SetIdentity, actor),
)?;
Ok(serde_json::json!({ "identity_updated": true }))
}
AdminOpBody::Invalidate { id, .. } => {
let ctx = AuditCtx::route_admin(AuditOp::Invalidate, actor);
let outcome = store.invalidate_and_revoke_all_audited(&id, ctx)?;
Expand Down Expand Up @@ -541,8 +606,8 @@ fn decode_hash32(s: &str) -> Option<[u8; 32]> {
pub enum StoreMode {
/// Create-only: fails if the id already exists.
Create,
/// Unconditional overwrite (version-guarded internally): the re-login / re-import
/// replace that keeps the handle.
/// Legacy unconditional overwrite. Its serialized shape remains frozen for old
/// clients; identity-aware replacements use `admin.store_with_identity_policy`.
ReplaceUnconditional,
/// CAS overwrite gated on the current payload hash (lowercase hex).
ReplaceCas { expected_hash_hex: String },
Expand Down Expand Up @@ -639,4 +704,31 @@ mod tests {
let s = String::from_utf8(op.to_bytes().unwrap()).unwrap();
assert!(s.contains("\"op\":\"admin.invalidate\""));
}

#[test]
fn legacy_unconditional_store_bytes_remain_compatible() {
let op = AdminOpBody::Store {
v: 1,
id: "apikey:x".into(),
record: Box::new(VaultRecord::new_static(
CredentialKind::ApiKey,
"t",
b"k".to_vec(),
None,
)),
audit_op: AdminAuditOp::Put,
mode: StoreMode::ReplaceUnconditional,
};
assert_eq!(
String::from_utf8(op.to_bytes().unwrap()).unwrap(),
"{\"op\":\"admin.store\",\"v\":1,\"id\":\"apikey:x\",\"record\":{\"schema_version\":1,\"kind\":\"api_key\",\"source\":\"t\",\"record_version\":1,\"expires_at_ms\":null,\"refresh_adapter\":null,\"oauth\":null,\"payload\":[107]},\"audit_op\":\"put\",\"mode\":{\"kind\":\"replace_unconditional\"}}"
);
}

#[test]
fn identity_policy_store_op_round_trips() {
let raw = b"{\"op\":\"admin.store_with_identity_policy\",\"v\":1,\"id\":\"apikey:x\",\"record\":{\"schema_version\":1,\"kind\":\"api_key\",\"source\":\"t\",\"record_version\":1,\"expires_at_ms\":null,\"refresh_adapter\":null,\"oauth\":null,\"payload\":[107]},\"audit_op\":\"put\",\"clear_identity\":false}";
let op: AdminOpBody = serde_json::from_slice(raw).expect("new policy op decodes");
assert_eq!(op.to_bytes().unwrap(), raw);
}
}
6 changes: 6 additions & 0 deletions crates/credentials-core/src/audit.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,9 @@ pub enum AuditOp {
Login,
/// An overwrite under a compare-and-set.
Overwrite,
/// A non-secret account identity update that re-seals the existing credential
/// material without replacing it.
SetIdentity,
/// An authoritative invalidate (revoke).
Invalidate,
/// A master-key rotation (rewrap).
Expand Down Expand Up @@ -100,6 +103,7 @@ impl AuditOp {
AuditOp::Import => "import",
AuditOp::Login => "login",
AuditOp::Overwrite => "overwrite",
AuditOp::SetIdentity => "set_identity",
AuditOp::Invalidate => "invalidate",
AuditOp::RotateMasterKey => "rotate_master_key",
AuditOp::RefreshCommit => "refresh_commit",
Expand Down Expand Up @@ -580,6 +584,7 @@ mod vocabulary_documentation_tests {
AuditOp::Import => AuditOp::Import.as_str(),
AuditOp::Login => AuditOp::Login.as_str(),
AuditOp::Overwrite => AuditOp::Overwrite.as_str(),
AuditOp::SetIdentity => AuditOp::SetIdentity.as_str(),
AuditOp::Invalidate => AuditOp::Invalidate.as_str(),
AuditOp::RotateMasterKey => AuditOp::RotateMasterKey.as_str(),
AuditOp::RefreshCommit => AuditOp::RefreshCommit.as_str(),
Expand All @@ -599,6 +604,7 @@ mod vocabulary_documentation_tests {
assert_documented(section, "audit_log.op", value(AuditOp::Import));
assert_documented(section, "audit_log.op", value(AuditOp::Login));
assert_documented(section, "audit_log.op", value(AuditOp::Overwrite));
assert_documented(section, "audit_log.op", value(AuditOp::SetIdentity));
assert_documented(section, "audit_log.op", value(AuditOp::Invalidate));
assert_documented(section, "audit_log.op", value(AuditOp::RotateMasterKey));
assert_documented(section, "audit_log.op", value(AuditOp::RefreshCommit));
Expand Down
84 changes: 75 additions & 9 deletions crates/credentials-core/src/record.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -136,6 +136,40 @@ pub struct RecordIdentity {
}

impl RecordIdentity {
/// Validate the bounded non-secret labels before a store write. Keeping this at
/// the record boundary prevents authenticated admin-op bytes from bypassing CLI
/// validation and making account grouping depend on malformed metadata.
pub fn validate(&self) -> Result<(), String> {
if let Some(account_id) = self.account_id.as_deref() {
if account_id.trim().is_empty() {
return Err("account_id must not be empty".to_string());
}
validate_identity_value("account_id", account_id)?;
}
if let Some(email) = self.email.as_deref() {
validate_identity_value("email", email)?;
}
if let Some(org_name) = self.org_name.as_deref() {
validate_identity_value("org_name", org_name)?;
}
Ok(())
}

/// Drop email-only identity before validation or persistence; consumers join on
/// account_id, so keeping its display-only counterpart would advertise a label
/// that cannot identify an account.
pub(crate) fn normalized(self) -> Self {
if self.is_servable() {
self
} else {
RecordIdentity {
account_id: None,
email: None,
org_name: self.org_name,
}
}
}

/// Whether this identity can be served without collapsing a consumer's labelling.
///
/// False for the one shape that looks captured and behaves as though it was not:
Expand All @@ -153,6 +187,19 @@ impl RecordIdentity {
}
}

fn validate_identity_value(field: &str, value: &str) -> Result<(), String> {
if value.trim().is_empty() {
return Err(format!("{field} must not be empty"));
}
if value.len() > 256 {
return Err(format!("{field} must be at most 256 bytes"));
}
if value.chars().any(char::is_control) {
return Err(format!("{field} must not contain control characters"));
}
Ok(())
}
Comment thread
cubic-dev-ai[bot] marked this conversation as resolved.

/// The vault's typed, at-rest view of one credential. Encrypted as one unit; only
/// `payload` is ever returned to a consumer.
#[derive(Clone, Serialize, Deserialize, PartialEq, Eq)]
Expand Down Expand Up @@ -279,15 +326,7 @@ impl VaultRecord {
/// Normalising beats refusing: an identity is display metadata, and failing a
/// login over it would trade a labelling gap for a lost credential.
pub fn with_identity(mut self, identity: RecordIdentity) -> Self {
self.identity = if identity.is_servable() {
identity
} else {
RecordIdentity {
account_id: None,
email: None,
org_name: identity.org_name,
}
};
self.identity = identity.normalized();
self
}

Expand Down Expand Up @@ -563,4 +602,31 @@ mod tests {
"\"cookie\""
);
}

#[test]
fn identity_validation_rejects_whitespace_only_optional_labels() {
for (field, identity) in [
(
"email",
RecordIdentity {
account_id: Some("acct".into()),
email: Some(" \t ".into()),
org_name: None,
},
),
(
"org_name",
RecordIdentity {
account_id: Some("acct".into()),
email: None,
org_name: Some(" ".into()),
},
),
] {
assert!(
identity.validate().is_err(),
"whitespace-only {field} must be rejected"
);
}
}
}
Loading