From 9c095b4275d4d9951d5a783be77591d515963e3f Mon Sep 17 00:00:00 2001 From: Henry Park Date: Tue, 23 Jun 2026 23:43:44 -0700 Subject: [PATCH 1/7] fix(reborn): populate provider on runtime auth-required gates MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A WASM/runtime capability whose injected credential returns 401 raises an `auth_required` gate with empty `credential_requirements` (`runtime_adapters.rs` `wasm_guest_dispatch_error`). That left `AuthPromptView.provider` null, so the WebUI manual-token card threw client-side (`useChat.submitAuthToken` requires `provider`) and never sent the submit — surfacing as "Could not save the token" with no network request. Enrich an empty `DispatchError::AuthRequired.credential_requirements` from the capability's already-declared credential obligations (`InjectCredentialAccountOnce` -> `RuntimeCredentialAuthRequirement`) in the capability host, where both the dispatch result and the obligations are in scope. Runtime-agnostic; never overrides a populated list. This reuses the same declared-requirement data the credential-missing path already surfaces, so re-auth gates become submittable. Adds a caller-level regression test driving `CapabilityHost::invoke_json` that asserts the gate carries the provider (fails before the fix), plus a non-override test. Co-Authored-By: Claude Opus 4.8 (1M context) --- crates/ironclaw_capabilities/src/host.rs | 64 ++++- ..._host_auth_required_enrichment_contract.rs | 234 ++++++++++++++++++ 2 files changed, 296 insertions(+), 2 deletions(-) create mode 100644 crates/ironclaw_capabilities/tests/capability_host_auth_required_enrichment_contract.rs diff --git a/crates/ironclaw_capabilities/src/host.rs b/crates/ironclaw_capabilities/src/host.rs index 10280079170..65d2c2ebc11 100644 --- a/crates/ironclaw_capabilities/src/host.rs +++ b/crates/ironclaw_capabilities/src/host.rs @@ -4,8 +4,9 @@ use ironclaw_authorization::{ use ironclaw_extensions::ExtensionRegistry; use ironclaw_host_api::{ CapabilityDescriptor, CapabilityDispatchRequest, CapabilityDispatchResult, - CapabilityDispatcher, CapabilityGrantId, CapabilityId, Decision, DenyReason, ExecutionContext, - InvocationFingerprint, InvocationId, Obligation, ProcessId, ResourceEstimate, ResourceScope, + CapabilityDispatcher, CapabilityGrantId, CapabilityId, Decision, DenyReason, DispatchError, + ExecutionContext, InvocationFingerprint, InvocationId, Obligation, ProcessId, ResourceEstimate, + ResourceScope, RuntimeCredentialAuthRequirement, }; use ironclaw_processes::{ProcessManager, ProcessStart}; use ironclaw_run_state::{ @@ -528,6 +529,7 @@ where &obligation_outcome, ) .await; + let error = enrich_auth_required_from_obligations(error, obligations.as_slice()); let invocation_error = CapabilityInvocationError::from(error); apply_run_state_transition_if_configured( self.run_state, @@ -1914,6 +1916,7 @@ where &obligation_outcome, ) .await; + let error = enrich_auth_required_from_obligations(error, obligations.as_slice()); let invocation_error = CapabilityInvocationError::from(error); apply_run_state_transition_if_configured( Some(run_state), @@ -2254,3 +2257,60 @@ fn obligation_invocation_error_kind(error: &CapabilityInvocationError) -> &'stat .map(CapabilityRunStateTransition::error_kind) .unwrap_or("Dispatch") } + +/// Enriches a `DispatchError::AuthRequired` with credential requirements derived +/// from the capability's declared `InjectCredentialAccountOnce` obligations when +/// the runtime returned an empty list. +/// +/// WASM extensions that signal `auth_required` after a 401 on an injected +/// credential produce an `AuthRequired` with empty `credential_requirements` +/// because the adapter only receives the error string, not the obligation list. +/// This function fills that gap so the auth-gate prompt carries the provider +/// identity the WebUI needs to build a submittable manual-token card. +/// +/// Only operates when `credential_requirements` is empty; a non-empty list from +/// the runtime (e.g. MCP passing through its own requirement set) is left +/// untouched. +fn enrich_auth_required_from_obligations( + error: DispatchError, + obligations: &[Obligation], +) -> DispatchError { + let DispatchError::AuthRequired { + capability, + required_secrets, + credential_requirements, + } = error + else { + return error; + }; + if !credential_requirements.is_empty() { + return DispatchError::AuthRequired { + capability, + required_secrets, + credential_requirements, + }; + } + let enriched: Vec = obligations + .iter() + .filter_map(|obligation| match obligation { + Obligation::InjectCredentialAccountOnce { + provider, + setup, + provider_scopes, + requester_extension, + .. + } => Some(RuntimeCredentialAuthRequirement { + provider: provider.clone(), + setup: setup.clone(), + requester_extension: requester_extension.clone(), + provider_scopes: provider_scopes.clone(), + }), + _ => None, + }) + .collect(); + DispatchError::AuthRequired { + capability, + required_secrets, + credential_requirements: enriched, + } +} diff --git a/crates/ironclaw_capabilities/tests/capability_host_auth_required_enrichment_contract.rs b/crates/ironclaw_capabilities/tests/capability_host_auth_required_enrichment_contract.rs new file mode 100644 index 00000000000..143e7ed908d --- /dev/null +++ b/crates/ironclaw_capabilities/tests/capability_host_auth_required_enrichment_contract.rs @@ -0,0 +1,234 @@ +// Regression tests for credential-requirement enrichment on dispatch-time AuthRequired. +// +// Bug: WASM extensions that signal `auth_required` after a 401 on an injected +// credential produced a `DispatchError::AuthRequired` with empty +// `credential_requirements` because the WASM adapter only receives the error +// string, not the obligation list. The WebUI's manual-token card inspects +// `provider` (derived from `credential_requirements`) and refused to send the +// network request when it was absent. +// +// Fix: `CapabilityHost::invoke_json` (and the shared dispatch-resumed tail) +// enriches empty `credential_requirements` from the capability's declared +// `InjectCredentialAccountOnce` obligations before converting the dispatch +// error into a `CapabilityInvocationError`. +// +// These tests drive `CapabilityHost::invoke_json` — the caller — rather than +// `enrich_auth_required_from_obligations` alone, so they cover the layer where +// the enrichment input (obligations) is silently dropped if the fix is absent. +// (See `.claude/rules/testing.md` "Test Through the Caller, Not Just the Helper".) +use async_trait::async_trait; +use ironclaw_authorization::TrustAwareCapabilityDispatchAuthorizer; +use ironclaw_capabilities::*; +use ironclaw_host_api::*; +use ironclaw_trust::TrustDecision; +use serde_json::json; + +mod support; +use support::*; + +// --------------------------------------------------------------------------- +// Stub: authorizer that returns Allow with an InjectCredentialAccountOnce +// obligation carrying a specific provider identity. +// --------------------------------------------------------------------------- + +struct CredentialObligationAuthorizer { + provider: RuntimeCredentialAccountProviderId, + setup: RuntimeCredentialAccountSetup, + requester_extension: ExtensionId, +} + +#[async_trait] +impl TrustAwareCapabilityDispatchAuthorizer for CredentialObligationAuthorizer { + async fn authorize_dispatch_with_trust( + &self, + _context: &ExecutionContext, + _descriptor: &CapabilityDescriptor, + _estimate: &ResourceEstimate, + _trust_decision: &TrustDecision, + ) -> Decision { + Decision::Allow { + obligations: Obligations::new(vec![Obligation::InjectCredentialAccountOnce { + handle: SecretHandle::new("github_pat").unwrap(), + provider: self.provider.clone(), + setup: self.setup.clone(), + provider_scopes: Vec::new(), + requester_extension: self.requester_extension.clone(), + }]) + .unwrap(), + } + } +} + +// --------------------------------------------------------------------------- +// Stub: obligation handler that accepts all obligations unconditionally (the +// failure under test happens at dispatch time, not obligation time). +// --------------------------------------------------------------------------- + +struct PassthroughObligationHandler; + +#[async_trait] +impl CapabilityObligationHandler for PassthroughObligationHandler { + async fn satisfy( + &self, + _request: CapabilityObligationRequest<'_>, + ) -> Result<(), CapabilityObligationError> { + Ok(()) + } +} + +// --------------------------------------------------------------------------- +// Stub: dispatcher that always returns AuthRequired with an empty +// credential_requirements list, simulating a WASM adapter that only knows the +// error string and has no access to the obligation list. +// --------------------------------------------------------------------------- + +struct AuthRequiredDispatcher; + +#[async_trait] +impl CapabilityDispatcher for AuthRequiredDispatcher { + async fn dispatch_json( + &self, + request: CapabilityDispatchRequest, + ) -> Result { + Err(DispatchError::AuthRequired { + capability: request.capability_id, + required_secrets: Vec::new(), + credential_requirements: Vec::new(), + }) + } +} + +// --------------------------------------------------------------------------- +// Test: invoke_json enriches empty credential_requirements from obligations +// --------------------------------------------------------------------------- + +#[tokio::test] +async fn invoke_json_enriches_auth_required_credential_requirements_from_obligations() { + let registry = registry_with_echo_capability(); + let provider = RuntimeCredentialAccountProviderId::new("github").unwrap(); + let requester = ExtensionId::new("github").unwrap(); + let authorizer = CredentialObligationAuthorizer { + provider: provider.clone(), + setup: RuntimeCredentialAccountSetup::ManualToken, + requester_extension: requester, + }; + let dispatcher = AuthRequiredDispatcher; + let handler = PassthroughObligationHandler; + let host = + CapabilityHost::new(®istry, &dispatcher, &authorizer).with_obligation_handler(&handler); + let context = execution_context(CapabilitySet { + grants: vec![dispatch_grant()], + }); + + let err = host + .invoke_json(CapabilityInvocationRequest { + context, + capability_id: capability_id(), + estimate: ResourceEstimate::default(), + input: json!({"owner": "acme", "repo": "api", "issue_number": 1, "body": "hi"}), + trust_decision: trust_decision(), + }) + .await + .unwrap_err(); + + let CapabilityInvocationError::AuthorizationRequiresAuth { + required_secrets, + credential_requirements, + .. + } = err + else { + panic!("expected AuthorizationRequiresAuth, got {err:?}"); + }; + assert!(required_secrets.is_empty()); + assert_eq!( + credential_requirements.len(), + 1, + "expected one credential requirement enriched from InjectCredentialAccountOnce obligation" + ); + assert_eq!( + credential_requirements[0].provider, provider, + "enriched requirement must carry the declared provider id" + ); + assert_eq!( + credential_requirements[0].setup, + RuntimeCredentialAccountSetup::ManualToken, + ); +} + +// --------------------------------------------------------------------------- +// Test: invoke_json does NOT overwrite non-empty credential_requirements +// --------------------------------------------------------------------------- + +#[tokio::test] +async fn invoke_json_preserves_non_empty_credential_requirements_from_dispatcher() { + // When the runtime already supplies requirements (e.g. MCP), the enrichment + // must not replace them. + struct AuthRequiredWithRequirementsDispatcher; + + #[async_trait] + impl CapabilityDispatcher for AuthRequiredWithRequirementsDispatcher { + async fn dispatch_json( + &self, + request: CapabilityDispatchRequest, + ) -> Result { + let mcp_provider = RuntimeCredentialAccountProviderId::new("mcp_provider").unwrap(); + let mcp_ext = ExtensionId::new("mcp_ext").unwrap(); + Err(DispatchError::AuthRequired { + capability: request.capability_id, + required_secrets: Vec::new(), + credential_requirements: vec![RuntimeCredentialAuthRequirement { + provider: mcp_provider, + setup: RuntimeCredentialAccountSetup::OAuth { scopes: Vec::new() }, + requester_extension: mcp_ext, + provider_scopes: Vec::new(), + }], + }) + } + } + + let registry = registry_with_echo_capability(); + let obligation_provider = RuntimeCredentialAccountProviderId::new("github").unwrap(); + let requester = ExtensionId::new("github").unwrap(); + let authorizer = CredentialObligationAuthorizer { + provider: obligation_provider, + setup: RuntimeCredentialAccountSetup::ManualToken, + requester_extension: requester, + }; + let dispatcher = AuthRequiredWithRequirementsDispatcher; + let handler = PassthroughObligationHandler; + let host = + CapabilityHost::new(®istry, &dispatcher, &authorizer).with_obligation_handler(&handler); + let context = execution_context(CapabilitySet { + grants: vec![dispatch_grant()], + }); + + let err = host + .invoke_json(CapabilityInvocationRequest { + context, + capability_id: capability_id(), + estimate: ResourceEstimate::default(), + input: json!({}), + trust_decision: trust_decision(), + }) + .await + .unwrap_err(); + + let CapabilityInvocationError::AuthorizationRequiresAuth { + credential_requirements, + .. + } = err + else { + panic!("expected AuthorizationRequiresAuth, got {err:?}"); + }; + assert_eq!( + credential_requirements.len(), + 1, + "non-empty runtime requirements must not be replaced by obligation enrichment" + ); + // The retained requirement must be the one from the dispatcher (mcp_provider), + // not the one from the obligation (github). + assert_eq!( + credential_requirements[0].provider, + RuntimeCredentialAccountProviderId::new("mcp_provider").unwrap(), + ); +} From de0bb1155e6e0432005a70adeeb6b20ca2af6388 Mon Sep 17 00:00:00 2001 From: Henry Park Date: Wed, 24 Jun 2026 00:10:03 -0700 Subject: [PATCH 2/7] refactor(host-api): move auth-requirement enrichment onto host_api types; fix take-one MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Address code-review findings on the runtime auth-required enrichment: - Correctness: emit at most one credential requirement. The downstream consumer (auth_prompt_from_credential_requirement) matches exactly one (`let [requirement] = ...`); emitting >1 for capabilities with multiple credential obligations made it fall through and leave the gate unsubmittable. Enrich with `.take(1)`. - Altitude/duplication: move the logic onto the types that own it in ironclaw_host_api — `Obligation::credential_auth_requirement()` and `DispatchError::enrich_auth_requirements(&[Obligation])`. Delete the free helper from the capability host (host.rs shrinks; the two call sites become one-liners and can't drift). - Tests: add resume-path coverage (auth_resume_json -> dispatch_resumed_ capability), a multi-obligation test locking the take-one contract, and host_api unit tests for both new methods. Co-Authored-By: Claude Opus 4.8 (1M context) --- crates/ironclaw_capabilities/src/host.rs | 66 +----- ..._host_auth_required_enrichment_contract.rs | 199 ++++++++++++++++++ crates/ironclaw_host_api/src/decision.rs | 72 +++++++ crates/ironclaw_host_api/src/dispatch.rs | 178 +++++++++++++++- 4 files changed, 451 insertions(+), 64 deletions(-) diff --git a/crates/ironclaw_capabilities/src/host.rs b/crates/ironclaw_capabilities/src/host.rs index 65d2c2ebc11..42a42122b6e 100644 --- a/crates/ironclaw_capabilities/src/host.rs +++ b/crates/ironclaw_capabilities/src/host.rs @@ -4,9 +4,8 @@ use ironclaw_authorization::{ use ironclaw_extensions::ExtensionRegistry; use ironclaw_host_api::{ CapabilityDescriptor, CapabilityDispatchRequest, CapabilityDispatchResult, - CapabilityDispatcher, CapabilityGrantId, CapabilityId, Decision, DenyReason, DispatchError, - ExecutionContext, InvocationFingerprint, InvocationId, Obligation, ProcessId, ResourceEstimate, - ResourceScope, RuntimeCredentialAuthRequirement, + CapabilityDispatcher, CapabilityGrantId, CapabilityId, Decision, DenyReason, ExecutionContext, + InvocationFingerprint, InvocationId, Obligation, ProcessId, ResourceEstimate, ResourceScope, }; use ironclaw_processes::{ProcessManager, ProcessStart}; use ironclaw_run_state::{ @@ -529,7 +528,7 @@ where &obligation_outcome, ) .await; - let error = enrich_auth_required_from_obligations(error, obligations.as_slice()); + let error = error.enrich_auth_requirements(obligations.as_slice()); let invocation_error = CapabilityInvocationError::from(error); apply_run_state_transition_if_configured( self.run_state, @@ -1916,7 +1915,7 @@ where &obligation_outcome, ) .await; - let error = enrich_auth_required_from_obligations(error, obligations.as_slice()); + let error = error.enrich_auth_requirements(obligations.as_slice()); let invocation_error = CapabilityInvocationError::from(error); apply_run_state_transition_if_configured( Some(run_state), @@ -2257,60 +2256,3 @@ fn obligation_invocation_error_kind(error: &CapabilityInvocationError) -> &'stat .map(CapabilityRunStateTransition::error_kind) .unwrap_or("Dispatch") } - -/// Enriches a `DispatchError::AuthRequired` with credential requirements derived -/// from the capability's declared `InjectCredentialAccountOnce` obligations when -/// the runtime returned an empty list. -/// -/// WASM extensions that signal `auth_required` after a 401 on an injected -/// credential produce an `AuthRequired` with empty `credential_requirements` -/// because the adapter only receives the error string, not the obligation list. -/// This function fills that gap so the auth-gate prompt carries the provider -/// identity the WebUI needs to build a submittable manual-token card. -/// -/// Only operates when `credential_requirements` is empty; a non-empty list from -/// the runtime (e.g. MCP passing through its own requirement set) is left -/// untouched. -fn enrich_auth_required_from_obligations( - error: DispatchError, - obligations: &[Obligation], -) -> DispatchError { - let DispatchError::AuthRequired { - capability, - required_secrets, - credential_requirements, - } = error - else { - return error; - }; - if !credential_requirements.is_empty() { - return DispatchError::AuthRequired { - capability, - required_secrets, - credential_requirements, - }; - } - let enriched: Vec = obligations - .iter() - .filter_map(|obligation| match obligation { - Obligation::InjectCredentialAccountOnce { - provider, - setup, - provider_scopes, - requester_extension, - .. - } => Some(RuntimeCredentialAuthRequirement { - provider: provider.clone(), - setup: setup.clone(), - requester_extension: requester_extension.clone(), - provider_scopes: provider_scopes.clone(), - }), - _ => None, - }) - .collect(); - DispatchError::AuthRequired { - capability, - required_secrets, - credential_requirements: enriched, - } -} diff --git a/crates/ironclaw_capabilities/tests/capability_host_auth_required_enrichment_contract.rs b/crates/ironclaw_capabilities/tests/capability_host_auth_required_enrichment_contract.rs index 143e7ed908d..67144896279 100644 --- a/crates/ironclaw_capabilities/tests/capability_host_auth_required_enrichment_contract.rs +++ b/crates/ironclaw_capabilities/tests/capability_host_auth_required_enrichment_contract.rs @@ -232,3 +232,202 @@ async fn invoke_json_preserves_non_empty_credential_requirements_from_dispatcher RuntimeCredentialAccountProviderId::new("mcp_provider").unwrap(), ); } + +// --------------------------------------------------------------------------- +// Test: dispatch_resumed_capability (second call site) also enriches +// +// Drives `invoke_json` → BlockedAuth → `auth_resume_json` where the +// dispatcher returns AuthRequired with empty credential_requirements on the +// resumed dispatch. Asserts that the enriched credential_requirements carry +// the provider declared by the authorizer's InjectCredentialAccountOnce +// obligation — proving the second call site in `dispatch_resumed_capability` +// is covered. +// --------------------------------------------------------------------------- + +#[tokio::test] +async fn auth_resume_json_enriches_auth_required_credential_requirements_from_obligations() { + use ironclaw_run_state::{InMemoryRunStateStore, RunStateStore, RunStatus}; + + // A dispatcher that returns AuthRequired with an empty credential_requirements + // list on every call (simulating a WASM adapter at both invoke and resume time). + struct AlwaysAuthRequiredDispatcher; + + #[async_trait] + impl CapabilityDispatcher for AlwaysAuthRequiredDispatcher { + async fn dispatch_json( + &self, + request: CapabilityDispatchRequest, + ) -> Result { + Err(DispatchError::AuthRequired { + capability: request.capability_id, + required_secrets: Vec::new(), + credential_requirements: Vec::new(), + }) + } + } + + let registry = registry_with_echo_capability(); + let provider = RuntimeCredentialAccountProviderId::new("github").unwrap(); + let requester = ExtensionId::new("github").unwrap(); + let authorizer = CredentialObligationAuthorizer { + provider: provider.clone(), + setup: RuntimeCredentialAccountSetup::ManualToken, + requester_extension: requester, + }; + let dispatcher = AlwaysAuthRequiredDispatcher; + let handler = PassthroughObligationHandler; + let run_state = InMemoryRunStateStore::new(); + + let host = CapabilityHost::new(®istry, &dispatcher, &authorizer) + .with_obligation_handler(&handler) + .with_run_state(&run_state); + + let context = execution_context(CapabilitySet { + grants: vec![dispatch_grant()], + }); + let scope = context.resource_scope.clone(); + let invocation_id = context.invocation_id; + + // Phase 1: invoke_json → blocked at auth. + let invoke_err = host + .invoke_json(CapabilityInvocationRequest { + context: context.clone(), + capability_id: capability_id(), + estimate: ResourceEstimate::default(), + input: serde_json::json!({"owner": "acme", "repo": "api", "issue_number": 1, "body": "hi"}), + trust_decision: trust_decision(), + }) + .await + .unwrap_err(); + + assert!( + matches!( + invoke_err, + CapabilityInvocationError::AuthorizationRequiresAuth { .. } + ), + "expected AuthorizationRequiresAuth from invoke_json, got {invoke_err:?}" + ); + + // Manually block the run so auth_resume_json can act on it. + let run = run_state.get(&scope, invocation_id).await.unwrap().unwrap(); + assert_eq!(run.status, RunStatus::BlockedAuth); + + // Phase 2: auth_resume_json → dispatcher returns AuthRequired again → + // dispatch_resumed_capability enriches from obligations. + let resume_err = host + .auth_resume_json(CapabilityAuthResumeRequest { + context, + capability_id: capability_id(), + estimate: ResourceEstimate::default(), + input: serde_json::json!({"owner": "acme", "repo": "api", "issue_number": 1, "body": "hi"}), + trust_decision: trust_decision(), + approval_request_id: None, + }) + .await + .unwrap_err(); + + let CapabilityInvocationError::AuthorizationRequiresAuth { + credential_requirements, + .. + } = resume_err + else { + panic!("expected AuthorizationRequiresAuth from auth_resume_json, got {resume_err:?}"); + }; + + assert_eq!( + credential_requirements.len(), + 1, + "resume path must enrich empty credential_requirements from InjectCredentialAccountOnce obligation" + ); + assert_eq!( + credential_requirements[0].provider, provider, + "enriched requirement on resume path must carry the declared provider id" + ); +} + +// --------------------------------------------------------------------------- +// Test: multiple InjectCredentialAccountOnce obligations → only one emitted +// +// When the authorizer declares two InjectCredentialAccountOnce obligations +// (different providers), the enriched list must have length 1, not 2. +// This locks the `.take(1)` contract: the downstream consumer +// `auth_prompt_from_credential_requirement` matches exactly one requirement; +// emitting two would cause it to fall through and leave `provider` as None. +// --------------------------------------------------------------------------- + +#[tokio::test] +async fn invoke_json_emits_at_most_one_requirement_when_multiple_obligations_declared() { + struct MultiObligationAuthorizer; + + #[async_trait] + impl TrustAwareCapabilityDispatchAuthorizer for MultiObligationAuthorizer { + async fn authorize_dispatch_with_trust( + &self, + _context: &ExecutionContext, + _descriptor: &CapabilityDescriptor, + _estimate: &ResourceEstimate, + _trust_decision: &ironclaw_trust::TrustDecision, + ) -> Decision { + Decision::Allow { + obligations: Obligations::new(vec![ + Obligation::InjectCredentialAccountOnce { + handle: SecretHandle::new("github_pat").unwrap(), + provider: RuntimeCredentialAccountProviderId::new("github").unwrap(), + setup: RuntimeCredentialAccountSetup::ManualToken, + provider_scopes: Vec::new(), + requester_extension: ExtensionId::new("github").unwrap(), + }, + Obligation::InjectCredentialAccountOnce { + handle: SecretHandle::new("gitlab_pat").unwrap(), + provider: RuntimeCredentialAccountProviderId::new("gitlab").unwrap(), + setup: RuntimeCredentialAccountSetup::ManualToken, + provider_scopes: Vec::new(), + requester_extension: ExtensionId::new("gitlab").unwrap(), + }, + ]) + .unwrap(), + } + } + } + + let registry = registry_with_echo_capability(); + let authorizer = MultiObligationAuthorizer; + let dispatcher = AuthRequiredDispatcher; + let handler = PassthroughObligationHandler; + let host = + CapabilityHost::new(®istry, &dispatcher, &authorizer).with_obligation_handler(&handler); + let context = execution_context(CapabilitySet { + grants: vec![dispatch_grant()], + }); + + let err = host + .invoke_json(CapabilityInvocationRequest { + context, + capability_id: capability_id(), + estimate: ResourceEstimate::default(), + input: serde_json::json!({}), + trust_decision: trust_decision(), + }) + .await + .unwrap_err(); + + let CapabilityInvocationError::AuthorizationRequiresAuth { + credential_requirements, + .. + } = err + else { + panic!("expected AuthorizationRequiresAuth, got {err:?}"); + }; + + assert_eq!( + credential_requirements.len(), + 1, + "must emit exactly one credential requirement even when two InjectCredentialAccountOnce \ + obligations are declared — the downstream auth_prompt consumer handles exactly one" + ); + assert_eq!( + credential_requirements[0].provider, + RuntimeCredentialAccountProviderId::new("github").unwrap(), + "must emit the first obligation's provider" + ); +} diff --git a/crates/ironclaw_host_api/src/decision.rs b/crates/ironclaw_host_api/src/decision.rs index a9d478ee410..fc4cc1296f2 100644 --- a/crates/ironclaw_host_api/src/decision.rs +++ b/crates/ironclaw_host_api/src/decision.rs @@ -79,6 +79,29 @@ pub enum Obligation { }, } +impl Obligation { + /// Maps an `InjectCredentialAccountOnce` obligation to the auth requirement + /// the WebUI manual-token card / auth_prompt consumer needs to surface the + /// correct provider and setup flow. All other obligation variants return `None`. + pub fn credential_auth_requirement(&self) -> Option { + match self { + Obligation::InjectCredentialAccountOnce { + provider, + setup, + provider_scopes, + requester_extension, + .. + } => Some(RuntimeCredentialAuthRequirement { + provider: provider.clone(), + setup: setup.clone(), + requester_extension: requester_extension.clone(), + provider_scopes: provider_scopes.clone(), + }), + _ => None, + } + } +} + #[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] pub struct RuntimeCredentialAuthRequirement { pub provider: RuntimeCredentialAccountProviderId, @@ -249,3 +272,52 @@ impl Obligation { } } } + +#[cfg(test)] +mod tests { + use super::*; + + fn github_credential_obligation() -> Obligation { + Obligation::InjectCredentialAccountOnce { + handle: SecretHandle::new("github_pat").unwrap(), + provider: RuntimeCredentialAccountProviderId::new("github").unwrap(), + setup: RuntimeCredentialAccountSetup::ManualToken, + provider_scopes: vec!["repo".to_string()], + requester_extension: ExtensionId::new("github").unwrap(), + } + } + + #[test] + fn credential_auth_requirement_maps_inject_credential_account_once() { + let obligation = github_credential_obligation(); + let req = obligation + .credential_auth_requirement() + .expect("InjectCredentialAccountOnce must produce Some"); + + assert_eq!( + req.provider, + RuntimeCredentialAccountProviderId::new("github").unwrap() + ); + assert_eq!(req.setup, RuntimeCredentialAccountSetup::ManualToken); + assert_eq!(req.requester_extension, ExtensionId::new("github").unwrap()); + assert_eq!(req.provider_scopes, vec!["repo".to_string()]); + } + + #[test] + fn credential_auth_requirement_returns_none_for_non_credential_obligations() { + let non_credential = [ + Obligation::AuditBefore, + Obligation::AuditAfter, + Obligation::RedactOutput, + Obligation::InjectSecretOnce { + handle: SecretHandle::new("some_secret").unwrap(), + }, + ]; + for obligation in &non_credential { + assert!( + obligation.credential_auth_requirement().is_none(), + "{obligation:?} must return None from credential_auth_requirement" + ); + } + } +} diff --git a/crates/ironclaw_host_api/src/dispatch.rs b/crates/ironclaw_host_api/src/dispatch.rs index beddbb16722..256d128e26d 100644 --- a/crates/ironclaw_host_api/src/dispatch.rs +++ b/crates/ironclaw_host_api/src/dispatch.rs @@ -11,8 +11,9 @@ use serde_json::Value; use thiserror::Error; use crate::{ - CapabilityId, ExtensionId, MountView, ResourceEstimate, ResourceReceipt, ResourceReservation, - ResourceScope, ResourceUsage, RuntimeCredentialAuthRequirement, RuntimeKind, SecretHandle, + CapabilityId, ExtensionId, MountView, Obligation, ResourceEstimate, ResourceReceipt, + ResourceReservation, ResourceScope, ResourceUsage, RuntimeCredentialAuthRequirement, + RuntimeKind, SecretHandle, }; /// Request for one already-authorized declared capability dispatch. @@ -402,6 +403,51 @@ impl DispatchError { } } + /// Enriches an `AuthRequired` error's empty `credential_requirements` list from + /// the capability's declared obligations. + /// + /// When a WASM adapter (or any runtime that only receives the error string) returns + /// `AuthRequired` with an empty `credential_requirements`, this method fills it + /// with the first matching `InjectCredentialAccountOnce` obligation. + /// + /// Only the **first** obligation is emitted (`.take(1)`) because the downstream + /// consumer — `auth_prompt_from_credential_requirement` in + /// `ironclaw_reborn_composition` — matches exactly one requirement via + /// `let [requirement] = credential_requirements else { return view; }`. Emitting + /// more than one would cause the match to fall through and leave `provider` as + /// `None`, making the gate unsubmittable. + /// + /// If `self` is not `AuthRequired`, or if `credential_requirements` is already + /// non-empty (e.g. passed through from an MCP runtime), `self` is returned + /// unchanged without any allocation. + pub fn enrich_auth_requirements(self, obligations: &[Obligation]) -> Self { + match &self { + Self::AuthRequired { + credential_requirements, + .. + } if credential_requirements.is_empty() => {} + _ => return self, + } + let Self::AuthRequired { + capability, + required_secrets, + .. + } = self + else { + unreachable!("matched AuthRequired with empty requirements above") + }; + let enriched = obligations + .iter() + .filter_map(Obligation::credential_auth_requirement) + .take(1) + .collect(); + Self::AuthRequired { + capability, + required_secrets, + credential_requirements: enriched, + } + } + /// Stable event-token string for the error, suitable for telemetry and structured logging. /// /// This is the single canonical source for dispatch error event tokens; crates should @@ -435,6 +481,10 @@ pub trait CapabilityDispatcher: Send + Sync { #[cfg(test)] mod tests { use super::*; + use crate::{ + ExtensionId, Obligation, RuntimeCredentialAccountProviderId, RuntimeCredentialAccountSetup, + SecretHandle, + }; #[test] fn dispatch_input_issue_builder_methods_round_trip_optional_fields() { @@ -452,4 +502,128 @@ mod tests { Some("/properties/schedule/oneOf/0/properties/kind") ); } + + fn auth_required_empty(cap: &str) -> DispatchError { + DispatchError::AuthRequired { + capability: CapabilityId::new(cap).unwrap(), + required_secrets: Vec::new(), + credential_requirements: Vec::new(), + } + } + + fn auth_required_with_provider(cap: &str, provider: &str) -> DispatchError { + DispatchError::AuthRequired { + capability: CapabilityId::new(cap).unwrap(), + required_secrets: Vec::new(), + credential_requirements: vec![RuntimeCredentialAuthRequirement { + provider: RuntimeCredentialAccountProviderId::new(provider).unwrap(), + setup: RuntimeCredentialAccountSetup::ManualToken, + requester_extension: ExtensionId::new(provider).unwrap(), + provider_scopes: Vec::new(), + }], + } + } + + fn inject_credential_obligation(provider: &str) -> Obligation { + Obligation::InjectCredentialAccountOnce { + handle: SecretHandle::new(format!("{provider}_pat")).unwrap(), + provider: RuntimeCredentialAccountProviderId::new(provider).unwrap(), + setup: RuntimeCredentialAccountSetup::ManualToken, + provider_scopes: Vec::new(), + requester_extension: ExtensionId::new(provider).unwrap(), + } + } + + // enrich_auth_requirements: empty → enriched with first obligation + #[test] + fn enrich_auth_requirements_fills_empty_from_obligation() { + let error = auth_required_empty("echo.say"); + let obligations = [inject_credential_obligation("github")]; + + let enriched = error.enrich_auth_requirements(&obligations); + + let DispatchError::AuthRequired { + credential_requirements, + .. + } = enriched + else { + panic!("expected AuthRequired"); + }; + assert_eq!(credential_requirements.len(), 1); + assert_eq!( + credential_requirements[0].provider, + RuntimeCredentialAccountProviderId::new("github").unwrap() + ); + } + + // enrich_auth_requirements: already-populated → returned unchanged + #[test] + fn enrich_auth_requirements_leaves_non_empty_unchanged() { + let error = auth_required_with_provider("echo.say", "mcp_provider"); + let obligations = [inject_credential_obligation("github")]; + + let unchanged = error.enrich_auth_requirements(&obligations); + + let DispatchError::AuthRequired { + credential_requirements, + .. + } = unchanged + else { + panic!("expected AuthRequired"); + }; + // Must retain the original mcp_provider, not be replaced by github. + assert_eq!(credential_requirements.len(), 1); + assert_eq!( + credential_requirements[0].provider, + RuntimeCredentialAccountProviderId::new("mcp_provider").unwrap() + ); + } + + // enrich_auth_requirements: two obligations → only the first is emitted (.take(1)) + #[test] + fn enrich_auth_requirements_take_one_when_multiple_obligations_declared() { + let error = auth_required_empty("echo.say"); + let obligations = [ + inject_credential_obligation("github"), + inject_credential_obligation("gitlab"), + ]; + + let enriched = error.enrich_auth_requirements(&obligations); + + let DispatchError::AuthRequired { + credential_requirements, + .. + } = enriched + else { + panic!("expected AuthRequired"); + }; + // Downstream consumer (auth_prompt_from_credential_requirement) handles exactly one + // requirement; emitting more would cause it to fall through and leave provider as None. + assert_eq!( + credential_requirements.len(), + 1, + "must emit exactly one requirement even with two InjectCredentialAccountOnce obligations" + ); + assert_eq!( + credential_requirements[0].provider, + RuntimeCredentialAccountProviderId::new("github").unwrap(), + "must emit the first obligation's provider" + ); + } + + // enrich_auth_requirements: non-AuthRequired variants are returned unchanged + #[test] + fn enrich_auth_requirements_is_noop_for_non_auth_required_variants() { + let error = DispatchError::UnknownCapability { + capability: CapabilityId::new("echo.say").unwrap(), + }; + let obligations = [inject_credential_obligation("github")]; + + let unchanged = error.enrich_auth_requirements(&obligations); + + assert!( + matches!(unchanged, DispatchError::UnknownCapability { .. }), + "non-AuthRequired variants must be returned unchanged" + ); + } } From 7c8333deccf218aa09c88b2e5ba4abee13447344 Mon Sep 17 00:00:00 2001 From: Henry Park Date: Wed, 24 Jun 2026 00:32:24 -0700 Subject: [PATCH 3/7] fix(reborn): owner-granularity scope check for manual-token/selection completion MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Folds the second link of runtime credential re-auth into this PR. Manual-token (and credential-selection) submit mints a fresh per-request `invocation_id`, so completing a flow that reconnects to a credential account created in an earlier flow failed `scope_matches` full-equality with CrossScopeDenied (HTTP 403) — the "Could not save the token" follow-on once the gate became submittable. This is #4935 defect A on the unbound/reusable path. - `complete_manual_token` and `complete_credential_selection` (product_auth_durable/flows.rs) now use `binding_scope_owns_account`: owner-granularity (tenant/user/agent/project hard-required, session + surface exact-matched) while ignoring the ephemeral invocation_id (and thread/mission, intentional for owner-reusable accounts). - Mirror the same fix in the in-memory fake (fakes.rs) so it cannot mask the divergence in unit tests. - Tests: cross-invocation reconnect succeeds (both paths); genuinely foreign owner still rejected; cross-session and cross-surface still rejected (path-partitioned on disk). Follow-ups (not in this PR): rename `binding_scope_owns_account` -> `scope_owns_account` (now used on unbound paths too); extract a shared completion-account validation helper to unify the three call sites. Co-Authored-By: Claude Opus 4.8 (1M context) --- crates/ironclaw_auth/src/fakes.rs | 17 +- .../src/product_auth_durable/flows.rs | 22 +- .../src/product_auth_durable/tests.rs | 377 ++++++++++++++++++ 3 files changed, 412 insertions(+), 4 deletions(-) diff --git a/crates/ironclaw_auth/src/fakes.rs b/crates/ironclaw_auth/src/fakes.rs index 9a9240fc8f7..97b9de939ef 100644 --- a/crates/ironclaw_auth/src/fakes.rs +++ b/crates/ironclaw_auth/src/fakes.rs @@ -305,7 +305,12 @@ impl AuthFlowManager for InMemoryAuthProductServices { .accounts .get(&input.credential_account_id) .ok_or(AuthProductError::CredentialMissing)?; - if !scope_matches(&flow_scope, &account.scope) || account.provider != flow_provider { + // Use owner-granularity for the scope check, mirroring the production + // durable path (`flows.rs`). The flow record may carry a different + // invocation_id/thread_id/mission_id than the credential account; only + // the ownership boundary (tenant/user/agent/project + surface + session) + // is meaningful here. See `binding_scope_owns_account` in credential.rs:580. + if !binding_scope_owns_account(&flow_scope, account) || account.provider != flow_provider { return Err(AuthProductError::CrossScopeDenied); } if account.status != CredentialAccountStatus::Configured { @@ -364,7 +369,15 @@ impl AuthFlowManager for InMemoryAuthProductServices { .accounts .get(&input.credential_account_id) .ok_or(AuthProductError::CredentialMissing)?; - if !scope_matches(&flow_scope, &account.scope) || account.provider != flow_provider { + // Use owner-granularity for the scope check, mirroring the production + // durable path (`flows.rs`). The flow record's scope carries a fresh + // per-request `invocation_id` while the credential account may have been + // created under a different `invocation_id` (and/or thread/mission) in an + // earlier flow. Full `scope_matches` equality would always fail across + // requests. The meaningful ownership boundary is enforced by + // `binding_scope_owns_account` (tenant/user/agent/project + surface + + // session); see the canonical docstring at credential.rs:580. + if !binding_scope_owns_account(&flow_scope, account) || account.provider != flow_provider { return Err(AuthProductError::CrossScopeDenied); } if account.status != CredentialAccountStatus::Configured { diff --git a/crates/ironclaw_reborn_composition/src/product_auth_durable/flows.rs b/crates/ironclaw_reborn_composition/src/product_auth_durable/flows.rs index f63d32bec98..42c7b377721 100644 --- a/crates/ironclaw_reborn_composition/src/product_auth_durable/flows.rs +++ b/crates/ironclaw_reborn_composition/src/product_auth_durable/flows.rs @@ -183,7 +183,15 @@ where .await? .map(|(account, _)| account) .ok_or(AuthProductError::CredentialMissing)?; - if !scope_matches(&record.scope, &account.scope) + // Use owner-granularity for the scope check (#4935 parity with + // complete_manual_token): the flow record may carry a different + // invocation_id/thread_id/mission_id than the credential account was + // originally created with. Full `scope_matches` equality would reject a + // legitimate cross-invocation selection. The meaningful ownership boundary + // (tenant/user/agent/project + surface + session) is enforced by + // `binding_scope_owns_account`; see the canonical docstring at + // crates/ironclaw_auth/src/credential.rs:580. + if !binding_scope_owns_account(&record.scope, &account) || account.provider != record.provider || account.status != CredentialAccountStatus::Configured { @@ -240,7 +248,17 @@ where .await? .map(|(account, _)| account) .ok_or(AuthProductError::CredentialMissing)?; - if !scope_matches(&record.scope, &account.scope) + // Use owner-granularity for the scope check (#4935 defect A, unbound/reusable path): + // the flow record's scope carries a fresh per-request `invocation_id` (minted + // by the submit handler for each HTTP call) while the credential account was + // created under a different `invocation_id`, `thread_id`, or `mission_id` in + // an earlier flow — all three are ephemeral and intentionally ignored for + // owner-reusable accounts. Full `scope_matches` equality would always fail + // across requests. The enforced ownership boundary is + // tenant/user/agent/project + surface + session; see the canonical docstring + // on `binding_scope_owns_account` at + // crates/ironclaw_auth/src/credential.rs:580. + if !binding_scope_owns_account(&record.scope, &account) || account.provider != record.provider || account.status != CredentialAccountStatus::Configured { diff --git a/crates/ironclaw_reborn_composition/src/product_auth_durable/tests.rs b/crates/ironclaw_reborn_composition/src/product_auth_durable/tests.rs index 416c51fdf15..4a4346c2f93 100644 --- a/crates/ironclaw_reborn_composition/src/product_auth_durable/tests.rs +++ b/crates/ironclaw_reborn_composition/src/product_auth_durable/tests.rs @@ -3601,3 +3601,380 @@ async fn filesystem_manual_token_consume_only_after_successful_account_write() { "interaction must be consumed after successful retry" ); } + +// ─── fix: complete_manual_token accepts reconnect across a fresh invocation_id + +#[tokio::test] +async fn filesystem_complete_manual_token_succeeds_across_different_invocation_id() { + // Regression for #4935 class, unbound/reusable completion path: + // `complete_manual_token` previously called `scope_matches` (full equality) + // to validate the credential account. The submit handler mints a fresh + // `invocation_id` on every HTTP request, so the flow record's scope differs + // from the credential account's scope by `invocation_id` alone. That full + // equality check caused `CrossScopeDenied` on every real re-auth attempt. + // + // After the fix the check uses `binding_scope_owns_account` (owner + // granularity: tenant/user/agent/project + surface + session, ignoring the + // ephemeral `invocation_id`), so a legitimate reconnect now succeeds. + // + // This test MUST FAIL before the fix (it will return CrossScopeDenied). + let filesystem = test_filesystem(); + let secret_store: Arc = Arc::new(InMemorySecretStore::new()); + + // Build an account scope whose invocation_id is A (the "earlier request"). + let mut account_resource = test_scope().resource; + account_resource.invocation_id = InvocationId::new(); + let account_scope = + AuthProductScope::new(account_resource.clone(), ironclaw_auth::AuthSurface::Web); + + // Build a flow-record scope whose invocation_id is B (a "later request"). + // All other fields are identical. + let mut flow_resource = account_resource.clone(); + flow_resource.invocation_id = InvocationId::new(); // fresh — B != A + let flow_scope = AuthProductScope::new(flow_resource.clone(), ironclaw_auth::AuthSurface::Web); + + let service = test_service(filesystem, secret_store); + let expires_at = Utc::now() + Duration::minutes(5); + + // Create the credential account under invocation A. + let account = service + .create_account(NewCredentialAccount { + scope: account_scope.clone(), + provider: google_provider(), + label: account_label(), + status: CredentialAccountStatus::Configured, + ownership: CredentialOwnership::UserReusable, + owner_extension: None, + granted_extensions: vec![], + access_secret: Some(SecretHandle::new("reauth-access").unwrap()), + refresh_secret: None, + scopes: vec![], + }) + .await + .unwrap(); + + // Create the manual-token flow under invocation B. + let interaction_id = create_manual_token_flow(&service, &flow_scope, expires_at).await; + + // Drive complete_manual_token with a scope built from invocation B. + // Before the fix this returned CrossScopeDenied; after the fix it succeeds. + let completed = service + .complete_manual_token( + &flow_scope, + ManualTokenCompletionInput { + interaction_id, + credential_account_id: account.id, + }, + ) + .await + .expect( + "complete_manual_token must succeed when only invocation_id differs (regression: \ + CrossScopeDenied was returned before the binding_scope_owns_account fix)", + ); + + assert_eq!( + completed.status, + AuthFlowStatus::Completed, + "flow must reach Completed status on cross-invocation reconnect" + ); + assert_eq!( + completed.credential_account_id, + Some(account.id), + "completed flow must reference the pre-existing credential account" + ); +} + +#[tokio::test] +async fn filesystem_complete_manual_token_still_rejects_genuinely_foreign_owner() { + // Ownership enforcement must NOT be relaxed by the fix: a flow whose record + // scope has a different *owner* (different user_id) than the credential account + // must still return CrossScopeDenied. This guards against + // `binding_scope_owns_account` being over-permissive. + let filesystem = test_filesystem(); + let secret_store: Arc = Arc::new(InMemorySecretStore::new()); + + // Build an account scope for user "bob". + let mut bob_resource = test_scope().resource; + bob_resource.user_id = UserId::new("bob").unwrap(); + let bob_scope = AuthProductScope::new(bob_resource, ironclaw_auth::AuthSurface::Web); + + // Build a flow scope for user "alice" (different owner). + let alice_scope = test_scope(); // alice's scope from the default helper + + let service = test_service(filesystem, secret_store); + let expires_at = Utc::now() + Duration::minutes(5); + + // Create an account owned by bob. + let bob_account = service + .create_account(NewCredentialAccount { + scope: bob_scope.clone(), + provider: google_provider(), + label: account_label(), + status: CredentialAccountStatus::Configured, + ownership: CredentialOwnership::UserReusable, + owner_extension: None, + granted_extensions: vec![], + access_secret: Some(SecretHandle::new("bob-access").unwrap()), + refresh_secret: None, + scopes: vec![], + }) + .await + .unwrap(); + + // Create the flow under alice's scope. + let interaction_id = create_manual_token_flow(&service, &alice_scope, expires_at).await; + + // Alice's flow must not be able to complete against bob's account. + let err = service + .complete_manual_token( + &alice_scope, + ManualTokenCompletionInput { + interaction_id, + credential_account_id: bob_account.id, + }, + ) + .await + .expect_err("completion against a foreign-owner account must return CrossScopeDenied"); + + assert_eq!( + err, + AuthProductError::CrossScopeDenied, + "owner-level boundary must still be enforced after the invocation_id fix" + ); +} + +// ─── security: enforced isolation axes — session and surface are exact-matched + +#[tokio::test] +async fn filesystem_complete_manual_token_rejects_different_session_id() { + // `binding_scope_owns_account` must still reject a credential account whose + // `session_id` differs from the flow record's session_id even when every + // other ownership axis (tenant/user/agent/project/surface) matches. + // This locks the "session is exact-matched" invariant documented in the + // `binding_scope_owns_account` docstring. + // + // This test MUST FAIL before fix #1 (same-session uses scope_matches which + // may pass, but here scope_matches would fail on session_id mismatch too — + // either way the new binding_scope_owns_account correctly enforces it). + let filesystem = test_filesystem(); + let secret_store: Arc = Arc::new(InMemorySecretStore::new()); + + // Account created under session S1. + let account_resource = test_scope().resource; + let mut account_scope = AuthProductScope::new(account_resource.clone(), AuthSurface::Web); + account_scope.session_id = Some(AuthSessionId::new("session-s1").unwrap()); + + // Flow created under session S2 (same user/agent/project/surface). + let mut flow_resource = test_scope().resource; + flow_resource.invocation_id = InvocationId::new(); // different invocation too (realistic) + let mut flow_scope = AuthProductScope::new(flow_resource, AuthSurface::Web); + flow_scope.session_id = Some(AuthSessionId::new("session-s2").unwrap()); + + let service = test_service(filesystem, secret_store); + let expires_at = Utc::now() + Duration::minutes(5); + + // Create the credential account under session S1. + let account = service + .create_account(NewCredentialAccount { + scope: account_scope.clone(), + provider: google_provider(), + label: account_label(), + status: CredentialAccountStatus::Configured, + ownership: CredentialOwnership::UserReusable, + owner_extension: None, + granted_extensions: vec![], + access_secret: Some(SecretHandle::new("s1-access").unwrap()), + refresh_secret: None, + scopes: vec![], + }) + .await + .unwrap(); + + // Create the manual-token flow under session S2. + let interaction_id = create_manual_token_flow(&service, &flow_scope, expires_at).await; + + // Cross-session completion must be rejected — session is exact-matched. + // Note: the durable store partitions account paths by session_id (see + // `surface_sessions_root`), so a lookup under S2 will not find an account + // created under S1. The observed outcome is `CredentialMissing` rather than + // `CrossScopeDenied`; both are secure — the cross-session account is + // inaccessible either way. + let err = service + .complete_manual_token( + &flow_scope, + ManualTokenCompletionInput { + interaction_id, + credential_account_id: account.id, + }, + ) + .await + .expect_err("complete_manual_token with different session_id must be rejected"); + + assert!( + matches!( + err, + AuthProductError::CredentialMissing | AuthProductError::CrossScopeDenied + ), + "cross-session completion must return CredentialMissing or CrossScopeDenied \ + (session_id is an exact-matched axis — different session is never accessible), \ + got: {err:?}" + ); +} + +#[tokio::test] +async fn filesystem_complete_manual_token_rejects_different_auth_surface() { + // `binding_scope_owns_account` must still reject a credential account whose + // `surface` differs from the flow record's surface even when every other + // ownership axis matches and session_id is None on both. + // This locks the "surface is exact-matched" invariant. + // + // Note: because accounts are partitioned by surface in the filesystem path + // layout (see `surface_sessions_root`), a cross-surface account lookup via + // `read_account(scope, id)` will not find the account at all and will return + // `CredentialMissing` rather than `CrossScopeDenied`. Both are acceptable + // secure outcomes; this test documents which one actually occurs. + let filesystem = test_filesystem(); + let secret_store: Arc = Arc::new(InMemorySecretStore::new()); + + // Account created under AuthSurface::Web. + let web_scope = test_scope(); // uses Web surface by default (see test_scope()) + + // Flow created under AuthSurface::Cli (same owner, different surface). + let cli_scope = AuthProductScope::new(test_scope().resource, AuthSurface::Cli); + + let service = test_service(filesystem, secret_store); + let expires_at = Utc::now() + Duration::minutes(5); + + // Create the credential account under Web surface. + let account = service + .create_account(NewCredentialAccount { + scope: web_scope.clone(), + provider: google_provider(), + label: account_label(), + status: CredentialAccountStatus::Configured, + ownership: CredentialOwnership::UserReusable, + owner_extension: None, + granted_extensions: vec![], + access_secret: Some(SecretHandle::new("web-access").unwrap()), + refresh_secret: None, + scopes: vec![], + }) + .await + .unwrap(); + + // Create the manual-token flow under Cli surface. + let interaction_id = create_manual_token_flow(&service, &cli_scope, expires_at).await; + + // Cross-surface completion must be rejected. The filesystem partitions + // accounts by surface, so the account is simply not found from the Cli + // surface path — CredentialMissing is the observed (secure) outcome. + let err = service + .complete_manual_token( + &cli_scope, + ManualTokenCompletionInput { + interaction_id, + credential_account_id: account.id, + }, + ) + .await + .expect_err("complete_manual_token with different AuthSurface must be rejected"); + + assert!( + matches!( + err, + AuthProductError::CredentialMissing | AuthProductError::CrossScopeDenied + ), + "cross-surface completion must return CredentialMissing or CrossScopeDenied, got: {err:?}" + ); +} + +#[tokio::test] +async fn filesystem_complete_credential_selection_succeeds_across_different_invocation_id() { + // Regression test for fix #2 (`complete_credential_selection` parity with + // `complete_manual_token`): when the flow record's scope differs from the + // credential account's scope ONLY in the ephemeral `invocation_id` + // (and/or `thread_id`/`mission_id`), `complete_credential_selection` must + // succeed. Before fix #2 it used `scope_matches` (full equality) which would + // return `CrossScopeDenied` on every real cross-invocation selection. + // + // This test MUST FAIL before fix #2. + use ironclaw_auth::{AuthFlowKind, CredentialSelectionInput}; + + let filesystem = test_filesystem(); + let secret_store: Arc = Arc::new(InMemorySecretStore::new()); + + // Account created under invocation A. + let mut account_resource = test_scope().resource; + account_resource.invocation_id = InvocationId::new(); + let account_scope = AuthProductScope::new(account_resource.clone(), AuthSurface::Web); + + // Flow created under invocation B (all other fields identical). + let mut flow_resource = account_resource.clone(); + flow_resource.invocation_id = InvocationId::new(); // B != A + let flow_scope = AuthProductScope::new(flow_resource, AuthSurface::Web); + + let service = test_service(filesystem, secret_store); + + // Create the credential account under invocation A. + let account = service + .create_account(NewCredentialAccount { + scope: account_scope.clone(), + provider: google_provider(), + label: account_label(), + status: CredentialAccountStatus::Configured, + ownership: CredentialOwnership::UserReusable, + owner_extension: None, + granted_extensions: vec![], + access_secret: Some(SecretHandle::new("sel-access").unwrap()), + refresh_secret: None, + scopes: vec![], + }) + .await + .unwrap(); + + // Create the account-selection flow under invocation B. + let flow = service + .create_flow(NewAuthFlow { + id: None, + scope: flow_scope.clone(), + kind: AuthFlowKind::IntegrationCredential, + provider: google_provider(), + challenge: AuthChallenge::AccountSelectionRequired { + provider: google_provider(), + accounts: vec![account.projection()], + }, + continuation: AuthContinuationRef::SetupOnly, + update_binding: None, + opaque_state_hash: None, + pkce_verifier_hash: None, + expires_at: Utc::now() + Duration::minutes(5), + }) + .await + .unwrap(); + + // Cross-invocation completion must succeed after fix #2. + let completed = service + .complete_credential_selection( + &flow_scope, + CredentialSelectionInput { + flow_id: flow.id, + credential_account_id: account.id, + }, + ) + .await + .expect( + "complete_credential_selection must succeed when only invocation_id differs \ + (regression: CrossScopeDenied was returned before the binding_scope_owns_account fix)", + ); + + assert_eq!( + completed.status, + AuthFlowStatus::Completed, + "flow must reach Completed status on cross-invocation selection" + ); + assert_eq!( + completed.credential_account_id, + Some(account.id), + "completed flow must reference the pre-existing credential account" + ); +} From ee84f89be99452327217b9af39b73563c8eeecdb Mon Sep 17 00:00:00 2001 From: Henry Park Date: Wed, 24 Jun 2026 08:31:40 -0700 Subject: [PATCH 4/7] refactor: move auth-requirement enrichment policy to capabilities; tighten condition Address PR #5180 review (thermo-nuclear + multi-agent): - Altitude: keep the neutral `Obligation::credential_auth_requirement` mapper in ironclaw_host_api, but move the enrichment POLICY out of `DispatchError::enrich_auth_requirements` (product-workflow cardinality has no place in the neutral vocab crate per its guardrail) into a private helper in ironclaw_capabilities. - Correctness: synthesize the auth-gate credential requirement ONLY when the runtime gave no auth signal of its own (both `required_secrets` and `credential_requirements` empty) AND the capability declares EXACTLY ONE credential obligation. Raw-secret gates (required_secrets populated) are no longer mis-prompted as product-auth; multi-credential capabilities no longer get a wrong-provider gate (was `.take(1)` guessing the first). - Tests: unit tests for all helper branches; updated the multi-obligation contract test to assert the gate is left unmodified (empty) rather than pointed at an arbitrary provider. Also adds durable rejection coverage for complete_credential_selection (foreign owner reaches binding_scope_owns_account -> CrossScopeDenied; session/ surface are path-partitioned -> CredentialMissing, guard exact-match is defense-in-depth). Co-Authored-By: Claude Opus 4.8 (1M context) --- crates/ironclaw_capabilities/src/host.rs | 241 ++++++++++++++- ..._host_auth_required_enrichment_contract.rs | 28 +- crates/ironclaw_host_api/src/dispatch.rs | 178 +---------- .../src/product_auth_durable/tests.rs | 290 ++++++++++++++++++ 4 files changed, 541 insertions(+), 196 deletions(-) diff --git a/crates/ironclaw_capabilities/src/host.rs b/crates/ironclaw_capabilities/src/host.rs index 42a42122b6e..8b907ae844e 100644 --- a/crates/ironclaw_capabilities/src/host.rs +++ b/crates/ironclaw_capabilities/src/host.rs @@ -4,8 +4,9 @@ use ironclaw_authorization::{ use ironclaw_extensions::ExtensionRegistry; use ironclaw_host_api::{ CapabilityDescriptor, CapabilityDispatchRequest, CapabilityDispatchResult, - CapabilityDispatcher, CapabilityGrantId, CapabilityId, Decision, DenyReason, ExecutionContext, - InvocationFingerprint, InvocationId, Obligation, ProcessId, ResourceEstimate, ResourceScope, + CapabilityDispatcher, CapabilityGrantId, CapabilityId, Decision, DenyReason, DispatchError, + ExecutionContext, InvocationFingerprint, InvocationId, Obligation, ProcessId, ResourceEstimate, + ResourceScope, }; use ironclaw_processes::{ProcessManager, ProcessStart}; use ironclaw_run_state::{ @@ -528,7 +529,8 @@ where &obligation_outcome, ) .await; - let error = error.enrich_auth_requirements(obligations.as_slice()); + let error = + enrich_dispatch_error_credential_requirements(error, obligations.as_slice()); let invocation_error = CapabilityInvocationError::from(error); apply_run_state_transition_if_configured( self.run_state, @@ -1915,7 +1917,8 @@ where &obligation_outcome, ) .await; - let error = error.enrich_auth_requirements(obligations.as_slice()); + let error = + enrich_dispatch_error_credential_requirements(error, obligations.as_slice()); let invocation_error = CapabilityInvocationError::from(error); apply_run_state_transition_if_configured( Some(run_state), @@ -2256,3 +2259,233 @@ fn obligation_invocation_error_kind(error: &CapabilityInvocationError) -> &'stat .map(CapabilityRunStateTransition::error_kind) .unwrap_or("Dispatch") } + +/// Synthesize the auth-gate credential requirement for a runtime `AuthRequired` +/// that carries no auth detail of its own (the WASM-style 401 case), from the +/// capability's declared credential obligation. +/// +/// Fires ONLY when the runtime gave no auth signal at all — both `required_secrets` +/// and `credential_requirements` empty — AND the capability declares EXACTLY ONE +/// credential obligation. A raw-secret-handle gate (`required_secrets` populated) +/// must not be turned into a product-auth provider prompt; and with multiple +/// credential obligations the failed credential cannot be attributed, so we leave +/// the gate unmodified rather than guess the wrong provider. The downstream WebUI +/// manual-token card consumes exactly one provider. +fn enrich_dispatch_error_credential_requirements( + error: DispatchError, + obligations: &[Obligation], +) -> DispatchError { + let DispatchError::AuthRequired { + ref required_secrets, + ref credential_requirements, + .. + } = error + else { + return error; + }; + if !required_secrets.is_empty() || !credential_requirements.is_empty() { + return error; + } + let derived: Vec<_> = obligations + .iter() + .filter_map(Obligation::credential_auth_requirement) + .collect(); + let [requirement] = derived.as_slice() else { + return error; // zero or >1 credential obligations: do not guess + }; + let DispatchError::AuthRequired { + capability, + required_secrets, + .. + } = error + else { + unreachable!("matched AuthRequired above") + }; + DispatchError::AuthRequired { + capability, + required_secrets, + credential_requirements: vec![requirement.clone()], + } +} + +#[cfg(test)] +mod tests { + use super::*; + use ironclaw_host_api::{ + CapabilityId, ExtensionId, Obligation, RuntimeCredentialAccountProviderId, + RuntimeCredentialAccountSetup, SecretHandle, + }; + + fn auth_required_empty(cap: &str) -> DispatchError { + DispatchError::AuthRequired { + capability: CapabilityId::new(cap).unwrap(), + required_secrets: Vec::new(), + credential_requirements: Vec::new(), + } + } + + fn auth_required_with_secrets(cap: &str) -> DispatchError { + DispatchError::AuthRequired { + capability: CapabilityId::new(cap).unwrap(), + required_secrets: vec![SecretHandle::new("raw_secret").unwrap()], + credential_requirements: Vec::new(), + } + } + + fn auth_required_with_provider(cap: &str, provider: &str) -> DispatchError { + use ironclaw_host_api::RuntimeCredentialAuthRequirement; + DispatchError::AuthRequired { + capability: CapabilityId::new(cap).unwrap(), + required_secrets: Vec::new(), + credential_requirements: vec![RuntimeCredentialAuthRequirement { + provider: RuntimeCredentialAccountProviderId::new(provider).unwrap(), + setup: RuntimeCredentialAccountSetup::ManualToken, + requester_extension: ExtensionId::new(provider).unwrap(), + provider_scopes: Vec::new(), + }], + } + } + + fn inject_credential_obligation(provider: &str) -> Obligation { + Obligation::InjectCredentialAccountOnce { + handle: SecretHandle::new(format!("{provider}_pat")).unwrap(), + provider: RuntimeCredentialAccountProviderId::new(provider).unwrap(), + setup: RuntimeCredentialAccountSetup::ManualToken, + provider_scopes: Vec::new(), + requester_extension: ExtensionId::new(provider).unwrap(), + } + } + + // WASM case: both empty + exactly one obligation → enriched with that provider. + #[test] + fn enrich_fills_empty_from_single_credential_obligation() { + let error = auth_required_empty("echo.say"); + let obligations = [inject_credential_obligation("github")]; + + let result = enrich_dispatch_error_credential_requirements(error, &obligations); + + let DispatchError::AuthRequired { + credential_requirements, + .. + } = result + else { + panic!("expected AuthRequired"); + }; + assert_eq!(credential_requirements.len(), 1); + assert_eq!( + credential_requirements[0].provider, + RuntimeCredentialAccountProviderId::new("github").unwrap() + ); + } + + // required_secrets populated → returned unchanged (raw-secret gate must not become product-auth prompt). + #[test] + fn enrich_leaves_required_secrets_populated_unchanged() { + let error = auth_required_with_secrets("echo.say"); + let obligations = [inject_credential_obligation("github")]; + + let result = enrich_dispatch_error_credential_requirements(error, &obligations); + + let DispatchError::AuthRequired { + required_secrets, + credential_requirements, + .. + } = result + else { + panic!("expected AuthRequired"); + }; + assert_eq!( + required_secrets.len(), + 1, + "required_secrets must be preserved" + ); + assert!( + credential_requirements.is_empty(), + "credential_requirements must remain empty when required_secrets are present" + ); + } + + // credential_requirements already populated → returned unchanged (e.g. MCP runtime already supplied requirements). + #[test] + fn enrich_leaves_non_empty_credential_requirements_unchanged() { + let error = auth_required_with_provider("echo.say", "mcp_provider"); + let obligations = [inject_credential_obligation("github")]; + + let result = enrich_dispatch_error_credential_requirements(error, &obligations); + + let DispatchError::AuthRequired { + credential_requirements, + .. + } = result + else { + panic!("expected AuthRequired"); + }; + assert_eq!(credential_requirements.len(), 1); + assert_eq!( + credential_requirements[0].provider, + RuntimeCredentialAccountProviderId::new("mcp_provider").unwrap(), + "original mcp_provider must be retained, not replaced by github" + ); + } + + // ZERO credential obligations → unchanged (empty result, not a guess). + #[test] + fn enrich_leaves_unchanged_when_zero_credential_obligations() { + let error = auth_required_empty("echo.say"); + let obligations: [Obligation; 0] = []; + + let result = enrich_dispatch_error_credential_requirements(error, &obligations); + + let DispatchError::AuthRequired { + credential_requirements, + .. + } = result + else { + panic!("expected AuthRequired"); + }; + assert!( + credential_requirements.is_empty(), + "zero obligations must leave credential_requirements empty" + ); + } + + // TWO credential obligations → NOT enriched (cannot attribute failure to one provider). + #[test] + fn enrich_leaves_unchanged_when_two_credential_obligations() { + let error = auth_required_empty("echo.say"); + let obligations = [ + inject_credential_obligation("github"), + inject_credential_obligation("gitlab"), + ]; + + let result = enrich_dispatch_error_credential_requirements(error, &obligations); + + let DispatchError::AuthRequired { + credential_requirements, + .. + } = result + else { + panic!("expected AuthRequired"); + }; + assert!( + credential_requirements.is_empty(), + "two obligations must leave credential_requirements empty — cannot attribute which provider failed" + ); + } + + // Non-AuthRequired variants returned unchanged. + #[test] + fn enrich_is_noop_for_non_auth_required_variants() { + let error = DispatchError::UnknownCapability { + capability: CapabilityId::new("echo.say").unwrap(), + }; + let obligations = [inject_credential_obligation("github")]; + + let result = enrich_dispatch_error_credential_requirements(error, &obligations); + + assert!( + matches!(result, DispatchError::UnknownCapability { .. }), + "non-AuthRequired variants must be returned unchanged" + ); + } +} diff --git a/crates/ironclaw_capabilities/tests/capability_host_auth_required_enrichment_contract.rs b/crates/ironclaw_capabilities/tests/capability_host_auth_required_enrichment_contract.rs index 67144896279..0765fd6c99d 100644 --- a/crates/ironclaw_capabilities/tests/capability_host_auth_required_enrichment_contract.rs +++ b/crates/ironclaw_capabilities/tests/capability_host_auth_required_enrichment_contract.rs @@ -346,17 +346,18 @@ async fn auth_resume_json_enriches_auth_required_credential_requirements_from_ob } // --------------------------------------------------------------------------- -// Test: multiple InjectCredentialAccountOnce obligations → only one emitted +// Test: multiple InjectCredentialAccountOnce obligations → NOT enriched // // When the authorizer declares two InjectCredentialAccountOnce obligations -// (different providers), the enriched list must have length 1, not 2. -// This locks the `.take(1)` contract: the downstream consumer -// `auth_prompt_from_credential_requirement` matches exactly one requirement; -// emitting two would cause it to fall through and leave `provider` as None. +// (different providers), the gate is left unmodified — credential_requirements +// stays EMPTY — because the failed credential cannot be attributed to one +// provider without guessing. Emitting the wrong provider would point the +// WebUI manual-token card at the wrong credential and make the gate +// unresolvable. // --------------------------------------------------------------------------- #[tokio::test] -async fn invoke_json_emits_at_most_one_requirement_when_multiple_obligations_declared() { +async fn invoke_json_does_not_enrich_when_multiple_credential_obligations_declared() { struct MultiObligationAuthorizer; #[async_trait] @@ -419,15 +420,10 @@ async fn invoke_json_emits_at_most_one_requirement_when_multiple_obligations_dec panic!("expected AuthorizationRequiresAuth, got {err:?}"); }; - assert_eq!( - credential_requirements.len(), - 1, - "must emit exactly one credential requirement even when two InjectCredentialAccountOnce \ - obligations are declared — the downstream auth_prompt consumer handles exactly one" - ); - assert_eq!( - credential_requirements[0].provider, - RuntimeCredentialAccountProviderId::new("github").unwrap(), - "must emit the first obligation's provider" + assert!( + credential_requirements.is_empty(), + "must NOT enrich when two InjectCredentialAccountOnce obligations are declared — \ + failed credential cannot be attributed to one provider; gate is left unmodified \ + rather than mis-pointed at the wrong provider" ); } diff --git a/crates/ironclaw_host_api/src/dispatch.rs b/crates/ironclaw_host_api/src/dispatch.rs index 256d128e26d..beddbb16722 100644 --- a/crates/ironclaw_host_api/src/dispatch.rs +++ b/crates/ironclaw_host_api/src/dispatch.rs @@ -11,9 +11,8 @@ use serde_json::Value; use thiserror::Error; use crate::{ - CapabilityId, ExtensionId, MountView, Obligation, ResourceEstimate, ResourceReceipt, - ResourceReservation, ResourceScope, ResourceUsage, RuntimeCredentialAuthRequirement, - RuntimeKind, SecretHandle, + CapabilityId, ExtensionId, MountView, ResourceEstimate, ResourceReceipt, ResourceReservation, + ResourceScope, ResourceUsage, RuntimeCredentialAuthRequirement, RuntimeKind, SecretHandle, }; /// Request for one already-authorized declared capability dispatch. @@ -403,51 +402,6 @@ impl DispatchError { } } - /// Enriches an `AuthRequired` error's empty `credential_requirements` list from - /// the capability's declared obligations. - /// - /// When a WASM adapter (or any runtime that only receives the error string) returns - /// `AuthRequired` with an empty `credential_requirements`, this method fills it - /// with the first matching `InjectCredentialAccountOnce` obligation. - /// - /// Only the **first** obligation is emitted (`.take(1)`) because the downstream - /// consumer — `auth_prompt_from_credential_requirement` in - /// `ironclaw_reborn_composition` — matches exactly one requirement via - /// `let [requirement] = credential_requirements else { return view; }`. Emitting - /// more than one would cause the match to fall through and leave `provider` as - /// `None`, making the gate unsubmittable. - /// - /// If `self` is not `AuthRequired`, or if `credential_requirements` is already - /// non-empty (e.g. passed through from an MCP runtime), `self` is returned - /// unchanged without any allocation. - pub fn enrich_auth_requirements(self, obligations: &[Obligation]) -> Self { - match &self { - Self::AuthRequired { - credential_requirements, - .. - } if credential_requirements.is_empty() => {} - _ => return self, - } - let Self::AuthRequired { - capability, - required_secrets, - .. - } = self - else { - unreachable!("matched AuthRequired with empty requirements above") - }; - let enriched = obligations - .iter() - .filter_map(Obligation::credential_auth_requirement) - .take(1) - .collect(); - Self::AuthRequired { - capability, - required_secrets, - credential_requirements: enriched, - } - } - /// Stable event-token string for the error, suitable for telemetry and structured logging. /// /// This is the single canonical source for dispatch error event tokens; crates should @@ -481,10 +435,6 @@ pub trait CapabilityDispatcher: Send + Sync { #[cfg(test)] mod tests { use super::*; - use crate::{ - ExtensionId, Obligation, RuntimeCredentialAccountProviderId, RuntimeCredentialAccountSetup, - SecretHandle, - }; #[test] fn dispatch_input_issue_builder_methods_round_trip_optional_fields() { @@ -502,128 +452,4 @@ mod tests { Some("/properties/schedule/oneOf/0/properties/kind") ); } - - fn auth_required_empty(cap: &str) -> DispatchError { - DispatchError::AuthRequired { - capability: CapabilityId::new(cap).unwrap(), - required_secrets: Vec::new(), - credential_requirements: Vec::new(), - } - } - - fn auth_required_with_provider(cap: &str, provider: &str) -> DispatchError { - DispatchError::AuthRequired { - capability: CapabilityId::new(cap).unwrap(), - required_secrets: Vec::new(), - credential_requirements: vec![RuntimeCredentialAuthRequirement { - provider: RuntimeCredentialAccountProviderId::new(provider).unwrap(), - setup: RuntimeCredentialAccountSetup::ManualToken, - requester_extension: ExtensionId::new(provider).unwrap(), - provider_scopes: Vec::new(), - }], - } - } - - fn inject_credential_obligation(provider: &str) -> Obligation { - Obligation::InjectCredentialAccountOnce { - handle: SecretHandle::new(format!("{provider}_pat")).unwrap(), - provider: RuntimeCredentialAccountProviderId::new(provider).unwrap(), - setup: RuntimeCredentialAccountSetup::ManualToken, - provider_scopes: Vec::new(), - requester_extension: ExtensionId::new(provider).unwrap(), - } - } - - // enrich_auth_requirements: empty → enriched with first obligation - #[test] - fn enrich_auth_requirements_fills_empty_from_obligation() { - let error = auth_required_empty("echo.say"); - let obligations = [inject_credential_obligation("github")]; - - let enriched = error.enrich_auth_requirements(&obligations); - - let DispatchError::AuthRequired { - credential_requirements, - .. - } = enriched - else { - panic!("expected AuthRequired"); - }; - assert_eq!(credential_requirements.len(), 1); - assert_eq!( - credential_requirements[0].provider, - RuntimeCredentialAccountProviderId::new("github").unwrap() - ); - } - - // enrich_auth_requirements: already-populated → returned unchanged - #[test] - fn enrich_auth_requirements_leaves_non_empty_unchanged() { - let error = auth_required_with_provider("echo.say", "mcp_provider"); - let obligations = [inject_credential_obligation("github")]; - - let unchanged = error.enrich_auth_requirements(&obligations); - - let DispatchError::AuthRequired { - credential_requirements, - .. - } = unchanged - else { - panic!("expected AuthRequired"); - }; - // Must retain the original mcp_provider, not be replaced by github. - assert_eq!(credential_requirements.len(), 1); - assert_eq!( - credential_requirements[0].provider, - RuntimeCredentialAccountProviderId::new("mcp_provider").unwrap() - ); - } - - // enrich_auth_requirements: two obligations → only the first is emitted (.take(1)) - #[test] - fn enrich_auth_requirements_take_one_when_multiple_obligations_declared() { - let error = auth_required_empty("echo.say"); - let obligations = [ - inject_credential_obligation("github"), - inject_credential_obligation("gitlab"), - ]; - - let enriched = error.enrich_auth_requirements(&obligations); - - let DispatchError::AuthRequired { - credential_requirements, - .. - } = enriched - else { - panic!("expected AuthRequired"); - }; - // Downstream consumer (auth_prompt_from_credential_requirement) handles exactly one - // requirement; emitting more would cause it to fall through and leave provider as None. - assert_eq!( - credential_requirements.len(), - 1, - "must emit exactly one requirement even with two InjectCredentialAccountOnce obligations" - ); - assert_eq!( - credential_requirements[0].provider, - RuntimeCredentialAccountProviderId::new("github").unwrap(), - "must emit the first obligation's provider" - ); - } - - // enrich_auth_requirements: non-AuthRequired variants are returned unchanged - #[test] - fn enrich_auth_requirements_is_noop_for_non_auth_required_variants() { - let error = DispatchError::UnknownCapability { - capability: CapabilityId::new("echo.say").unwrap(), - }; - let obligations = [inject_credential_obligation("github")]; - - let unchanged = error.enrich_auth_requirements(&obligations); - - assert!( - matches!(unchanged, DispatchError::UnknownCapability { .. }), - "non-AuthRequired variants must be returned unchanged" - ); - } } diff --git a/crates/ironclaw_reborn_composition/src/product_auth_durable/tests.rs b/crates/ironclaw_reborn_composition/src/product_auth_durable/tests.rs index 4a4346c2f93..15f81d857e6 100644 --- a/crates/ironclaw_reborn_composition/src/product_auth_durable/tests.rs +++ b/crates/ironclaw_reborn_composition/src/product_auth_durable/tests.rs @@ -3690,6 +3690,14 @@ async fn filesystem_complete_manual_token_still_rejects_genuinely_foreign_owner( // scope has a different *owner* (different user_id) than the credential account // must still return CrossScopeDenied. This guards against // `binding_scope_owns_account` being over-permissive. + // + // GUARD ANALYSIS: `user_id` is NOT encoded in the on-disk path (the path is + // keyed by surface + session, not by user; the filesystem mount is fixed to + // alice's tree in tests). Bob's account written via `create_account` lands at + // the SAME physical path that alice's flow reads. Therefore `read_account` + // returns `Some(bob_account)`, and the `CrossScopeDenied` comes from + // `binding_scope_owns_account` comparing the scopes — the guard itself is + // exercised, not a path-partition miss. let filesystem = test_filesystem(); let secret_store: Arc = Arc::new(InMemorySecretStore::new()); @@ -3978,3 +3986,285 @@ async fn filesystem_complete_credential_selection_succeeds_across_different_invo "completed flow must reference the pre-existing credential account" ); } + +// ─── security: complete_credential_selection ownership enforcement ──────────── + +#[tokio::test] +async fn filesystem_complete_credential_selection_rejects_genuinely_foreign_owner() { + // Reviewer A (serrrfirat): `complete_credential_selection` must enforce the + // same ownership boundary as `complete_manual_token`. A flow owned by alice + // must not complete against a credential account owned by bob, even after the + // `binding_scope_owns_account` relaxation for ephemeral invocation_id/thread. + // + // GUARD ANALYSIS: `user_id` is NOT encoded in the on-disk account path (path + // is keyed by surface + session only; the test filesystem mount is fixed to + // alice's tree). Bob's account therefore lands at the same physical path that + // alice's flow reads — `read_account` returns `Some(bob_account)`. The + // `CrossScopeDenied` comes from `binding_scope_owns_account` itself (the guard + // is exercised, not a path-partition miss). This is the most important new + // test: it proves the guard actually fires on a reachable foreign-owner account. + use ironclaw_auth::{AuthFlowKind, CredentialSelectionInput}; + + let filesystem = test_filesystem(); + let secret_store: Arc = Arc::new(InMemorySecretStore::new()); + + // Account created under user "bob" (foreign owner). + let mut bob_resource = test_scope().resource; + bob_resource.user_id = UserId::new("bob").unwrap(); + let bob_scope = AuthProductScope::new(bob_resource, AuthSurface::Web); + + // Flow created under user "alice" (the default `test_scope()`). + let alice_scope = test_scope(); + + let service = test_service(filesystem, secret_store); + + // Create a Configured account owned by bob. + let bob_account = service + .create_account(NewCredentialAccount { + scope: bob_scope.clone(), + provider: google_provider(), + label: account_label(), + status: CredentialAccountStatus::Configured, + ownership: CredentialOwnership::UserReusable, + owner_extension: None, + granted_extensions: vec![], + access_secret: Some(SecretHandle::new("bob-sel-access").unwrap()), + refresh_secret: None, + scopes: vec![], + }) + .await + .unwrap(); + + // Create the account-selection flow under alice's scope, advertising bob's + // account id (simulates a tampered or confused client submission). + let flow = service + .create_flow(NewAuthFlow { + id: None, + scope: alice_scope.clone(), + kind: AuthFlowKind::IntegrationCredential, + provider: google_provider(), + challenge: AuthChallenge::AccountSelectionRequired { + provider: google_provider(), + accounts: vec![bob_account.projection()], + }, + continuation: AuthContinuationRef::SetupOnly, + update_binding: None, + opaque_state_hash: None, + pkce_verifier_hash: None, + expires_at: Utc::now() + Duration::minutes(5), + }) + .await + .unwrap(); + + // Alice's flow must not complete against bob's account — CrossScopeDenied. + let err = service + .complete_credential_selection( + &alice_scope, + CredentialSelectionInput { + flow_id: flow.id, + credential_account_id: bob_account.id, + }, + ) + .await + .expect_err( + "complete_credential_selection against a foreign-owner account must return \ + CrossScopeDenied", + ); + + assert_eq!( + err, + AuthProductError::CrossScopeDenied, + "binding_scope_owns_account must reject a reachable account whose user_id differs \ + from the flow scope's user_id" + ); +} + +#[tokio::test] +async fn filesystem_complete_credential_selection_rejects_different_session_id() { + // Reviewer A (serrrfirat) parity with `complete_manual_token` session test. + // `complete_credential_selection` must reject an attempt to complete a + // selection flow whose scope carries session S2 against a credential account + // created under session S1. + // + // GUARD ANALYSIS: `session_id` IS encoded in the on-disk account path (see + // `product_auth_root` — the path includes `/sessions/{session_id}` when + // `session_id` is Some). An account stored under S1 is therefore NOT + // accessible from a read under S2. The durable store returns `None` for the + // account lookup → `CredentialMissing`. Both `CredentialMissing` (path + // partitioning intercepts before the guard) and `CrossScopeDenied` (the guard + // fires) are correct secure outcomes; this test locks which one actually + // occurs so it cannot silently regress. The `binding_scope_owns_account` + // session exact-match is defense-in-depth for any future code path that + // bypasses the path partitioning. + use ironclaw_auth::{AuthFlowKind, CredentialSelectionInput}; + + let filesystem = test_filesystem(); + let secret_store: Arc = Arc::new(InMemorySecretStore::new()); + + // Account created under session S1. + let account_resource = test_scope().resource; + let mut account_scope = AuthProductScope::new(account_resource.clone(), AuthSurface::Web); + account_scope.session_id = Some(AuthSessionId::new("sel-session-s1").unwrap()); + + // Flow created under session S2 (same surface, same owner, different session). + let mut flow_resource = test_scope().resource; + flow_resource.invocation_id = InvocationId::new(); // realistic fresh invocation + let mut flow_scope = AuthProductScope::new(flow_resource, AuthSurface::Web); + flow_scope.session_id = Some(AuthSessionId::new("sel-session-s2").unwrap()); + + let service = test_service(filesystem, secret_store); + + // Create the credential account under session S1. + let account = service + .create_account(NewCredentialAccount { + scope: account_scope.clone(), + provider: google_provider(), + label: account_label(), + status: CredentialAccountStatus::Configured, + ownership: CredentialOwnership::UserReusable, + owner_extension: None, + granted_extensions: vec![], + access_secret: Some(SecretHandle::new("sel-s1-access").unwrap()), + refresh_secret: None, + scopes: vec![], + }) + .await + .unwrap(); + + // Create the account-selection flow under session S2. + let flow = service + .create_flow(NewAuthFlow { + id: None, + scope: flow_scope.clone(), + kind: AuthFlowKind::IntegrationCredential, + provider: google_provider(), + challenge: AuthChallenge::AccountSelectionRequired { + provider: google_provider(), + accounts: vec![account.projection()], + }, + continuation: AuthContinuationRef::SetupOnly, + update_binding: None, + opaque_state_hash: None, + pkce_verifier_hash: None, + expires_at: Utc::now() + Duration::minutes(5), + }) + .await + .unwrap(); + + // Cross-session completion must be rejected. + // The disk layout partitions by session_id so the account is not found at + // all under S2 → CredentialMissing. CrossScopeDenied would be returned if + // the account were somehow reachable with a mismatched session. Both are + // correct secure outcomes; accepting either documents the actual behavior. + let err = service + .complete_credential_selection( + &flow_scope, + CredentialSelectionInput { + flow_id: flow.id, + credential_account_id: account.id, + }, + ) + .await + .expect_err("complete_credential_selection with different session_id must be rejected"); + + assert!( + matches!( + err, + AuthProductError::CredentialMissing | AuthProductError::CrossScopeDenied + ), + "cross-session credential selection must return CredentialMissing (path-partition \ + intercepts before the guard) or CrossScopeDenied (guard fires on a reachable \ + session-mismatched account), got: {err:?}" + ); +} + +#[tokio::test] +async fn filesystem_complete_credential_selection_rejects_different_auth_surface() { + // Reviewer A (serrrfirat) parity with `complete_manual_token` surface test. + // `complete_credential_selection` must reject an attempt to complete a + // selection flow whose scope carries surface Cli against a credential account + // created under surface Web. + // + // GUARD ANALYSIS: `surface` IS encoded in the on-disk account path (see + // `surface_path_segment` in `paths.rs`). An account stored under Web is NOT + // accessible from a read under Cli — `read_account` returns `None` → + // `CredentialMissing`. The `binding_scope_owns_account` surface exact-match is + // defense-in-depth: if a future refactor bypasses path partitioning the guard + // would catch a reachable surface-mismatched account and return + // `CrossScopeDenied`. Both outcomes are correct and secure; this test locks + // which one occurs so a regression cannot pass silently. + use ironclaw_auth::{AuthFlowKind, CredentialSelectionInput}; + + let filesystem = test_filesystem(); + let secret_store: Arc = Arc::new(InMemorySecretStore::new()); + + // Account created under AuthSurface::Web (default from test_scope()). + let web_scope = test_scope(); + + // Flow created under AuthSurface::Cli (same owner, different surface). + let cli_scope = AuthProductScope::new(test_scope().resource, AuthSurface::Cli); + + let service = test_service(filesystem, secret_store); + + // Create the credential account under Web surface. + let account = service + .create_account(NewCredentialAccount { + scope: web_scope.clone(), + provider: google_provider(), + label: account_label(), + status: CredentialAccountStatus::Configured, + ownership: CredentialOwnership::UserReusable, + owner_extension: None, + granted_extensions: vec![], + access_secret: Some(SecretHandle::new("sel-web-access").unwrap()), + refresh_secret: None, + scopes: vec![], + }) + .await + .unwrap(); + + // Create the account-selection flow under Cli surface. + let flow = service + .create_flow(NewAuthFlow { + id: None, + scope: cli_scope.clone(), + kind: AuthFlowKind::IntegrationCredential, + provider: google_provider(), + challenge: AuthChallenge::AccountSelectionRequired { + provider: google_provider(), + accounts: vec![account.projection()], + }, + continuation: AuthContinuationRef::SetupOnly, + update_binding: None, + opaque_state_hash: None, + pkce_verifier_hash: None, + expires_at: Utc::now() + Duration::minutes(5), + }) + .await + .unwrap(); + + // Cross-surface completion must be rejected. + // The filesystem partitions by surface path segment so the account is not + // found from Cli → CredentialMissing. CrossScopeDenied would fire if the + // account were somehow reachable with a mismatched surface. + let err = service + .complete_credential_selection( + &cli_scope, + CredentialSelectionInput { + flow_id: flow.id, + credential_account_id: account.id, + }, + ) + .await + .expect_err("complete_credential_selection with different AuthSurface must be rejected"); + + assert!( + matches!( + err, + AuthProductError::CredentialMissing | AuthProductError::CrossScopeDenied + ), + "cross-surface credential selection must return CredentialMissing (path-partition \ + intercepts before the guard) or CrossScopeDenied (guard fires on a reachable \ + surface-mismatched account), got: {err:?}" + ); +} From 0cfe1a22a002326f486185e93ee6e35355d616e6 Mon Sep 17 00:00:00 2001 From: Henry Park Date: Wed, 24 Jun 2026 21:18:46 -0700 Subject: [PATCH 5/7] test(reborn): fix runtime-401 reauth-gate contract; cover guard session/surface MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Addresses the CI failure and the remaining review feedback on the credential_requirements enrichment PR. - github_wasm_runtime_contract.rs: the two google-drive WASM 401 tests asserted credential_requirements.is_empty() — the pre-fix, un-wired contract from #4969 (provider-null, unsubmittable gate, #5174). The enrichment now populates the gate from the single credential obligation, so assert one requirement with provider=google + OAuth setup. This is the runtime-401 re-auth fallback; proactive refresh (inline + background keepalive) already runs before injection. - host.rs: document the reactive-refresh-on-runtime-401 follow-up on the enrichment helper, and correct the downstream-consumer note (OAuth setup launches the OAuth flow, ManualToken renders the token card). - credential.rs: add direct unit tests for binding_scope_owns_account covering the session_id and surface exact-match branches. The durable filesystem caller tests partition account records by surface+session path, so those axes only ever returned CredentialMissing and never executed the guard's equality branches (coderabbit review point). Co-Authored-By: Claude Opus 4.8 (1M context) --- crates/ironclaw_auth/src/credential.rs | 153 ++++++++++++++++++ crates/ironclaw_capabilities/src/host.rs | 13 +- .../tests/github_wasm_runtime_contract.rs | 35 +++- 3 files changed, 198 insertions(+), 3 deletions(-) diff --git a/crates/ironclaw_auth/src/credential.rs b/crates/ironclaw_auth/src/credential.rs index 235fe4f69de..7652c093d42 100644 --- a/crates/ironclaw_auth/src/credential.rs +++ b/crates/ironclaw_auth/src/credential.rs @@ -1068,3 +1068,156 @@ fn recovery_kind_and_reason_for_status( ), } } + +#[cfg(test)] +mod tests { + use super::*; + use crate::{ + AuthProviderId, AuthSessionId, AuthSurface, CredentialAccountId, CredentialAccountLabel, + CredentialAccountStatus, ProviderScope, scope::AuthProductScope, + }; + use chrono::Utc; + use ironclaw_host_api::{InvocationId, ResourceScope, UserId}; + + /// Build a minimal CredentialAccount using the same idiom as domain.rs tests. + fn make_account(scope: AuthProductScope) -> CredentialAccount { + CredentialAccount { + id: CredentialAccountId::new(), + scope, + provider: AuthProviderId::new("github").unwrap(), + label: CredentialAccountLabel::new("github-account").unwrap(), + status: CredentialAccountStatus::Configured, + ownership: CredentialOwnership::UserReusable, + owner_extension: None, + granted_extensions: Vec::new(), + access_secret: None, + refresh_secret: None, + scopes: vec![ProviderScope::new("read").unwrap()], + created_at: Utc::now(), + updated_at: Utc::now(), + } + } + + /// Build a base ResourceScope for a known owner with a given invocation_id. + fn owner_resource(invocation_id: InvocationId) -> ResourceScope { + ResourceScope::local_default(UserId::new("alice").unwrap(), invocation_id).unwrap() + } + + // Case 1: all axes match, including surface and session. invocation_id differs + // between the flow scope and the account scope to prove invocation_id is ignored + // (the exact-match invariant applies only to session_id and surface). + #[test] + fn binding_scope_owns_account_returns_true_when_all_axes_match() { + let session = AuthSessionId::new("ses-abc").unwrap(); + + // Account was created in an earlier flow with invocation_id A. + let account_scope = + AuthProductScope::new(owner_resource(InvocationId::new()), AuthSurface::Web) + .with_session_id(session.clone()); + let account = make_account(account_scope); + + // Current reconnect flow has a fresh invocation_id B — should still own. + let flow_scope = + AuthProductScope::new(owner_resource(InvocationId::new()), AuthSurface::Web) + .with_session_id(session); + + assert!( + binding_scope_owns_account(&flow_scope, &account), + "same owner/surface/session with differing invocation_id must return true" + ); + } + + // Case 2: owner matches and surface matches, but session_id differs. + // Exact-match invariant on session_id must reject the binding. + #[test] + fn binding_scope_owns_account_returns_false_when_session_differs() { + let account_scope = + AuthProductScope::new(owner_resource(InvocationId::new()), AuthSurface::Web) + .with_session_id(AuthSessionId::new("session-s1").unwrap()); + let account = make_account(account_scope); + + let flow_scope = + AuthProductScope::new(owner_resource(InvocationId::new()), AuthSurface::Web) + .with_session_id(AuthSessionId::new("session-s2").unwrap()); + + assert!( + !binding_scope_owns_account(&flow_scope, &account), + "mismatched session_id must return false" + ); + } + + // Case 3: owner matches and session matches, but surface differs. + // Exact-match invariant on surface must reject the binding. + #[test] + fn binding_scope_owns_account_returns_false_when_surface_differs() { + let session = AuthSessionId::new("ses-xyz").unwrap(); + + let account_scope = + AuthProductScope::new(owner_resource(InvocationId::new()), AuthSurface::Web) + .with_session_id(session.clone()); + let account = make_account(account_scope); + + // Same owner and session, but the flow comes from the Chat surface. + let flow_scope = + AuthProductScope::new(owner_resource(InvocationId::new()), AuthSurface::Chat) + .with_session_id(session); + + assert!( + !binding_scope_owns_account(&flow_scope, &account), + "mismatched surface must return false" + ); + } + + // Case 4a: account has Some session, flow scope has None. + // session_id is compared with as_ref() equality, so Some(..) != None => false. + #[test] + fn binding_scope_owns_account_returns_false_when_account_has_session_but_scope_does_not() { + let account_scope = + AuthProductScope::new(owner_resource(InvocationId::new()), AuthSurface::Api) + .with_session_id(AuthSessionId::new("ses-present").unwrap()); + let account = make_account(account_scope); + + // Flow scope carries no session_id. + let flow_scope = + AuthProductScope::new(owner_resource(InvocationId::new()), AuthSurface::Api); + + assert!( + !binding_scope_owns_account(&flow_scope, &account), + "account Some(session) vs scope None must return false" + ); + } + + // Case 4b: flow scope has Some session, account has None. + // same as_ref() equality: Some(..) != None => false. + #[test] + fn binding_scope_owns_account_returns_false_when_scope_has_session_but_account_does_not() { + let account_scope = + AuthProductScope::new(owner_resource(InvocationId::new()), AuthSurface::Api); + let account = make_account(account_scope); + + let flow_scope = + AuthProductScope::new(owner_resource(InvocationId::new()), AuthSurface::Api) + .with_session_id(AuthSessionId::new("ses-present").unwrap()); + + assert!( + !binding_scope_owns_account(&flow_scope, &account), + "scope Some(session) vs account None must return false" + ); + } + + // Case 4c: both scope and account have None session — None == None => true. + #[test] + fn binding_scope_owns_account_returns_true_when_both_sessions_are_none() { + let account_scope = + AuthProductScope::new(owner_resource(InvocationId::new()), AuthSurface::Api); + let account = make_account(account_scope); + + let flow_scope = + AuthProductScope::new(owner_resource(InvocationId::new()), AuthSurface::Api); + + assert!( + binding_scope_owns_account(&flow_scope, &account), + "None session on both sides must return true (None == None)" + ); + } +} diff --git a/crates/ironclaw_capabilities/src/host.rs b/crates/ironclaw_capabilities/src/host.rs index 8b907ae844e..65e57bc5ebd 100644 --- a/crates/ironclaw_capabilities/src/host.rs +++ b/crates/ironclaw_capabilities/src/host.rs @@ -2270,7 +2270,18 @@ fn obligation_invocation_error_kind(error: &CapabilityInvocationError) -> &'stat /// must not be turned into a product-auth provider prompt; and with multiple /// credential obligations the failed credential cannot be attributed, so we leave /// the gate unmodified rather than guess the wrong provider. The downstream WebUI -/// manual-token card consumes exactly one provider. +/// auth surface consumes exactly one provider (manual-token card for +/// `ManualToken` setup, OAuth launch for `OAuth` setup). +/// +/// FOLLOW-UP (reactive OAuth refresh on runtime 401): for an `OAuth` credential +/// this gate is the *fallback* after refresh is exhausted — proactive refresh +/// already runs inline at injection (within the 5-min expiry margin) and via the +/// background keepalive worker. A runtime 401 still slips through when the token +/// looked fresh by `expires_at` but was revoked mid-life, where one reactive +/// "refresh + retry" before surfacing the gate would recover silently. That +/// retry does not exist today (pre-existing gap, not introduced here); the gate +/// remains correct for the genuinely-revoked case. Track as a resolver/egress +/// enhancement, not a change to this enrichment. fn enrich_dispatch_error_credential_requirements( error: DispatchError, obligations: &[Obligation], diff --git a/crates/ironclaw_host_runtime/tests/github_wasm_runtime_contract.rs b/crates/ironclaw_host_runtime/tests/github_wasm_runtime_contract.rs index f4bfc6b5318..7a315d318f8 100644 --- a/crates/ironclaw_host_runtime/tests/github_wasm_runtime_contract.rs +++ b/crates/ironclaw_host_runtime/tests/github_wasm_runtime_contract.rs @@ -409,7 +409,24 @@ async fn host_runtime_services_maps_google_drive_wasm_401_to_auth_required() { RuntimeCapabilityOutcome::AuthRequired(gate) => { assert_eq!(gate.capability_id, capability_id); assert!(gate.required_secrets.is_empty()); - assert!(gate.credential_requirements.is_empty()); + // The runtime 401 carries no auth detail of its own; the capability + // host enriches the gate from the single credential obligation so the + // WebUI can launch the google OAuth re-auth flow. An empty list here is + // the provider-null, unsubmittable gate (#5174). Inline/background + // refresh already ran before injection; a runtime 401 is the genuine + // re-auth fallback, so the gate must surface provider + OAuth setup. + assert_eq!(gate.credential_requirements.len(), 1); + let requirement = &gate.credential_requirements[0]; + assert_eq!( + requirement.provider, + RuntimeCredentialAccountProviderId::new("google").unwrap() + ); + assert_eq!( + requirement.setup, + ironclaw_host_api::RuntimeCredentialAccountSetup::OAuth { + scopes: vec!["https://www.googleapis.com/auth/drive.readonly".to_string()] + } + ); } other => panic!("expected auth-required outcome, got {other:?}"), } @@ -471,7 +488,21 @@ async fn host_runtime_services_maps_google_drive_upload_wasm_401_to_auth_require RuntimeCapabilityOutcome::AuthRequired(gate) => { assert_eq!(gate.capability_id, capability_id); assert!(gate.required_secrets.is_empty()); - assert!(gate.credential_requirements.is_empty()); + // See list_files counterpart above: enrichment surfaces the single + // credential obligation's provider + OAuth setup so the re-auth gate is + // submittable (#5174). Empty would be the regressed provider-null gate. + assert_eq!(gate.credential_requirements.len(), 1); + let requirement = &gate.credential_requirements[0]; + assert_eq!( + requirement.provider, + RuntimeCredentialAccountProviderId::new("google").unwrap() + ); + assert_eq!( + requirement.setup, + ironclaw_host_api::RuntimeCredentialAccountSetup::OAuth { + scopes: vec!["https://www.googleapis.com/auth/drive".to_string()] + } + ); } other => panic!("expected auth-required outcome, got {other:?}"), } From 5c8219a3ae7d5570050a1b2e7db5fbc4d6b25ec3 Mon Sep 17 00:00:00 2001 From: Henry Park Date: Wed, 24 Jun 2026 21:21:02 -0700 Subject: [PATCH 6/7] docs(reborn): fix stale symbol/line refs in auth scope comments Address two low-severity review nits: - capability_host_auth_required_enrichment_contract.rs: header referenced the old helper name enrich_auth_required_from_obligations; rename to the real enrich_dispatch_error_credential_requirements. - fakes.rs / flows.rs: scope comments hard-coded credential.rs:580, which is already stale (binding_scope_owns_account is now at line 607). Drop the line number and point at the symbol only. Co-Authored-By: Claude Opus 4.8 (1M context) --- crates/ironclaw_auth/src/fakes.rs | 5 +++-- .../capability_host_auth_required_enrichment_contract.rs | 2 +- .../src/product_auth_durable/flows.rs | 4 ++-- 3 files changed, 6 insertions(+), 5 deletions(-) diff --git a/crates/ironclaw_auth/src/fakes.rs b/crates/ironclaw_auth/src/fakes.rs index 97b9de939ef..7b9066a2bb8 100644 --- a/crates/ironclaw_auth/src/fakes.rs +++ b/crates/ironclaw_auth/src/fakes.rs @@ -309,7 +309,7 @@ impl AuthFlowManager for InMemoryAuthProductServices { // durable path (`flows.rs`). The flow record may carry a different // invocation_id/thread_id/mission_id than the credential account; only // the ownership boundary (tenant/user/agent/project + surface + session) - // is meaningful here. See `binding_scope_owns_account` in credential.rs:580. + // is meaningful here. See `binding_scope_owns_account` in credential.rs. if !binding_scope_owns_account(&flow_scope, account) || account.provider != flow_provider { return Err(AuthProductError::CrossScopeDenied); } @@ -376,7 +376,8 @@ impl AuthFlowManager for InMemoryAuthProductServices { // earlier flow. Full `scope_matches` equality would always fail across // requests. The meaningful ownership boundary is enforced by // `binding_scope_owns_account` (tenant/user/agent/project + surface + - // session); see the canonical docstring at credential.rs:580. + // session); see the canonical docstring on `binding_scope_owns_account` + // in credential.rs. if !binding_scope_owns_account(&flow_scope, account) || account.provider != flow_provider { return Err(AuthProductError::CrossScopeDenied); } diff --git a/crates/ironclaw_capabilities/tests/capability_host_auth_required_enrichment_contract.rs b/crates/ironclaw_capabilities/tests/capability_host_auth_required_enrichment_contract.rs index 0765fd6c99d..bceaf120bc8 100644 --- a/crates/ironclaw_capabilities/tests/capability_host_auth_required_enrichment_contract.rs +++ b/crates/ironclaw_capabilities/tests/capability_host_auth_required_enrichment_contract.rs @@ -13,7 +13,7 @@ // error into a `CapabilityInvocationError`. // // These tests drive `CapabilityHost::invoke_json` — the caller — rather than -// `enrich_auth_required_from_obligations` alone, so they cover the layer where +// `enrich_dispatch_error_credential_requirements` alone, so they cover the layer where // the enrichment input (obligations) is silently dropped if the fix is absent. // (See `.claude/rules/testing.md` "Test Through the Caller, Not Just the Helper".) use async_trait::async_trait; diff --git a/crates/ironclaw_reborn_composition/src/product_auth_durable/flows.rs b/crates/ironclaw_reborn_composition/src/product_auth_durable/flows.rs index 42c7b377721..e0c80a453b3 100644 --- a/crates/ironclaw_reborn_composition/src/product_auth_durable/flows.rs +++ b/crates/ironclaw_reborn_composition/src/product_auth_durable/flows.rs @@ -190,7 +190,7 @@ where // legitimate cross-invocation selection. The meaningful ownership boundary // (tenant/user/agent/project + surface + session) is enforced by // `binding_scope_owns_account`; see the canonical docstring at - // crates/ironclaw_auth/src/credential.rs:580. + // crates/ironclaw_auth/src/credential.rs. if !binding_scope_owns_account(&record.scope, &account) || account.provider != record.provider || account.status != CredentialAccountStatus::Configured @@ -257,7 +257,7 @@ where // across requests. The enforced ownership boundary is // tenant/user/agent/project + surface + session; see the canonical docstring // on `binding_scope_owns_account` at - // crates/ironclaw_auth/src/credential.rs:580. + // crates/ironclaw_auth/src/credential.rs. if !binding_scope_owns_account(&record.scope, &account) || account.provider != record.provider || account.status != CredentialAccountStatus::Configured From 2bbe78ef0161f9959972bf9995138236a738830a Mon Sep 17 00:00:00 2001 From: Henry Park Date: Wed, 24 Jun 2026 21:54:58 -0700 Subject: [PATCH 7/7] test(capabilities): cover raw-secret gate preservation; soften refresh doc MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Address two review nits on the enrichment helper: - host.rs: the OAuth runtime-401 follow-up doc stated cross-layer refresh (inline injection + background keepalive) as a guarantee, but those live in other crates and are not enforced here. Soften to "may already have been attempted". - capability_host_auth_required_enrichment_contract.rs: add invoke_json_preserves_required_secrets_from_dispatcher — a caller-level test driving CapabilityHost::invoke_json with a raw-secret AuthRequired (required_secrets populated, credential_requirements empty) while an InjectCredentialAccountOnce obligation is declared, asserting the gate is left unmodified (secrets preserved, not rewritten into a provider prompt). Previously covered only at the private-helper level. Co-Authored-By: Claude Opus 4.8 (1M context) --- crates/ironclaw_capabilities/src/host.rs | 4 +- ..._host_auth_required_enrichment_contract.rs | 84 +++++++++++++++++++ 2 files changed, 86 insertions(+), 2 deletions(-) diff --git a/crates/ironclaw_capabilities/src/host.rs b/crates/ironclaw_capabilities/src/host.rs index 65e57bc5ebd..198f8cfb4a9 100644 --- a/crates/ironclaw_capabilities/src/host.rs +++ b/crates/ironclaw_capabilities/src/host.rs @@ -2275,8 +2275,8 @@ fn obligation_invocation_error_kind(error: &CapabilityInvocationError) -> &'stat /// /// FOLLOW-UP (reactive OAuth refresh on runtime 401): for an `OAuth` credential /// this gate is the *fallback* after refresh is exhausted — proactive refresh -/// already runs inline at injection (within the 5-min expiry margin) and via the -/// background keepalive worker. A runtime 401 still slips through when the token +/// may already have been attempted inline at injection (within the 5-min expiry +/// margin) or by the background keepalive worker. A runtime 401 still slips through when the token /// looked fresh by `expires_at` but was revoked mid-life, where one reactive /// "refresh + retry" before surfacing the gate would recover silently. That /// retry does not exist today (pre-existing gap, not introduced here); the gate diff --git a/crates/ironclaw_capabilities/tests/capability_host_auth_required_enrichment_contract.rs b/crates/ironclaw_capabilities/tests/capability_host_auth_required_enrichment_contract.rs index bceaf120bc8..cd48b33f18e 100644 --- a/crates/ironclaw_capabilities/tests/capability_host_auth_required_enrichment_contract.rs +++ b/crates/ironclaw_capabilities/tests/capability_host_auth_required_enrichment_contract.rs @@ -427,3 +427,87 @@ async fn invoke_json_does_not_enrich_when_multiple_credential_obligations_declar rather than mis-pointed at the wrong provider" ); } + +// --------------------------------------------------------------------------- +// Test: invoke_json does NOT enrich when required_secrets is already populated +// +// Regression guard: the enrichment helper bails out when `required_secrets` is +// populated, but earlier caller-level tests only exercise the empty-required_secrets +// path. A future wiring regression in `CapabilityHost::invoke_json` that strips +// the raw-secret gate and re-derives it as a provider prompt would not be caught +// without a test that drives the caller with a pre-populated `required_secrets`. +// +// The authorizer DOES declare an InjectCredentialAccountOnce obligation so the +// test proves the preservation is due to the populated `required_secrets` check, +// not merely an absence of obligations. +// --------------------------------------------------------------------------- + +#[tokio::test] +async fn invoke_json_preserves_required_secrets_from_dispatcher() { + // A dispatcher that returns AuthRequired with required_secrets POPULATED + // and credential_requirements EMPTY — the raw-secret-handle gate case. + struct AuthRequiredWithSecretsDispatcher; + + #[async_trait] + impl CapabilityDispatcher for AuthRequiredWithSecretsDispatcher { + async fn dispatch_json( + &self, + request: CapabilityDispatchRequest, + ) -> Result { + Err(DispatchError::AuthRequired { + capability: request.capability_id, + required_secrets: vec![SecretHandle::new("raw_secret_handle").unwrap()], + credential_requirements: Vec::new(), + }) + } + } + + let registry = registry_with_echo_capability(); + let obligation_provider = RuntimeCredentialAccountProviderId::new("github").unwrap(); + let requester = ExtensionId::new("github").unwrap(); + // Authorizer declares an InjectCredentialAccountOnce obligation — enrichment + // WOULD fire on an empty gate, but must be suppressed here because + // required_secrets is already populated. + let authorizer = CredentialObligationAuthorizer { + provider: obligation_provider, + setup: RuntimeCredentialAccountSetup::ManualToken, + requester_extension: requester, + }; + let dispatcher = AuthRequiredWithSecretsDispatcher; + let handler = PassthroughObligationHandler; + let host = + CapabilityHost::new(®istry, &dispatcher, &authorizer).with_obligation_handler(&handler); + let context = execution_context(CapabilitySet { + grants: vec![dispatch_grant()], + }); + + let err = host + .invoke_json(CapabilityInvocationRequest { + context, + capability_id: capability_id(), + estimate: ResourceEstimate::default(), + input: json!({}), + trust_decision: trust_decision(), + }) + .await + .unwrap_err(); + + let CapabilityInvocationError::AuthorizationRequiresAuth { + required_secrets, + credential_requirements, + .. + } = err + else { + panic!("expected AuthorizationRequiresAuth, got {err:?}"); + }; + assert_eq!( + required_secrets.len(), + 1, + "required_secrets from dispatcher must be preserved when non-empty" + ); + assert!( + credential_requirements.is_empty(), + "credential_requirements must remain empty when required_secrets are present \ + — enrichment from obligations must be suppressed" + ); +}