fix(auth): expand policy variables in Resource and NotResource

crates/auth/src/policy/condition.rs

@@ -32,15 +32,33 @@ use super::matcher::wildcard_match;
 ///   Absent key is vacuously true.
 /// - `ForAnyValue`: at least one context value must match some policy value.
 ///   Absent key is false (unless wrapped in `IfExists`).
+///
+/// Policy variables in the values are read as in a `2012-10-17` document;
+/// [`evaluate_condition_with`] takes the document's choice.
 pub fn evaluate_condition(condition: &Condition, context: &impl ConditionContext) -> bool {
+    evaluate_condition_with(condition, true, context)
+}
+
+/// [`evaluate_condition`] for a statement of a document whose `Version` is
+/// `2012-10-17` when `version_2012` is true
+/// ([`super::document::PolicyDocument::expands_policy_variables`]).
+///
+/// The value says how the text between `${` and `}` in a condition value is
+/// read ([`reads_iam_variable_syntax`]); `${key}` expands whatever it is.
+pub fn evaluate_condition_with(
+    condition: &Condition,
+    version_2012: bool,
+    context: &impl ConditionContext,
+) -> bool {
     let context_values = context.resolve_key(&condition.key);
     let is_multivalued = context.is_multivalued_key(&condition.key);
+    let iam_syntax = reads_iam_variable_syntax(&condition.operator, version_2012);
 
     // Expand policy variables (e.g. `${aws:PrincipalTag/Team}`) in condition values.
     let expanded_values: Vec<String> = condition
         .values
         .iter()
-        .map(|v| expand_policy_variables(v, context))
+        .map(|v| expand_policy_variables(v, iam_syntax, context))
         .collect();
 
     match &condition.operator {
@@ -179,17 +197,24 @@ pub enum ConditionMatch {
 ///
 /// Returns [`ConditionMatch::Unresolvable`] when the key is unresolved and the
 /// context reports it as service-populated (case 3), and when a condition value
-/// reads such a key through a `${name}` policy variable, since the variable has
-/// no value to expand to. Otherwise returns `Match` or `NoMatch` from the
-/// ordinary evaluation, so cases 1 and 2 are unchanged.
+/// reads such a key through a `${name}` policy variable, or through a
+/// `${name, 'default'}` one where [`reads_iam_variable_syntax`] holds, since
+/// the variable has no value to expand to and the service, which has one,
+/// would not use the default. Otherwise returns `Match` or `NoMatch` from the
+/// ordinary evaluation, so cases 1 and 2 are unchanged. `version_2012` is as
+/// in [`evaluate_condition_with`].
 pub fn evaluate_condition_match(
     condition: &Condition,
+    version_2012: bool,
     context: &impl ConditionContext,
 ) -> ConditionMatch {
-    if is_unresolvable(&condition.key, context) || reads_unresolvable_variable(condition, context) {
+    let iam_syntax = reads_iam_variable_syntax(&condition.operator, version_2012);
+    if is_unresolvable(&condition.key, context)
+        || reads_unresolvable_variable(condition, iam_syntax, context)
+    {
         return ConditionMatch::Unresolvable;
     }
-    if evaluate_condition(condition, context) {
+    if evaluate_condition_with(condition, version_2012, context) {
         ConditionMatch::Match
     } else {
         ConditionMatch::NoMatch
@@ -202,42 +227,76 @@ fn is_unresolvable(key: &str, context: &impl ConditionContext) -> bool {
     context.resolve_key(key).is_none() && context.is_unresolvable_key(key)
 }
 
-/// Whether a value of `condition` names, as a `${name}` policy variable, a key
-/// [`is_unresolvable`] reports.
-fn reads_unresolvable_variable(condition: &Condition, context: &impl ConditionContext) -> bool {
-    condition
-        .values
-        .iter()
-        .any(|value| policy_variable_names(value).any(|name| is_unresolvable(name, context)))
+/// Whether a value of `condition` names, as a `${name}` or `${name, 'default'}`
+/// policy variable, a key [`is_unresolvable`] reports.
+///
+/// The names come from [`super::variables::variable_key_names`], which reads
+/// the text between the braces the way [`expand_policy_variables`] expands
+/// it with the same `iam_syntax`, so with it a default written beside such a
+/// key does not hide the key from this check.
+fn reads_unresolvable_variable(
+    condition: &Condition,
+    iam_syntax: bool,
+    context: &impl ConditionContext,
+) -> bool {
+    condition.values.iter().any(|value| {
+        super::variables::variable_key_names(value, iam_syntax)
+            .any(|name| is_unresolvable(name, context))
+    })
 }
 
-/// The names of the `${name}` policy variables in `value`, in the order
-/// [`expand_policy_variables`] reads them. A `${` with no closing brace names
-/// nothing, as it expands to itself.
-fn policy_variable_names(value: &str) -> impl Iterator<Item = &str> {
-    let mut rest = value;
-    std::iter::from_fn(move || {
-        let start = rest.find("${")?;
-        let after_start = &rest[start + 2..];
-        let end = after_start.find('}')?;
-        rest = &after_start[end + 1..];
-        Some(&after_start[..end])
-    })
+/// Whether the values of a condition under `operator`, in a document whose
+/// `Version` is `2012-10-17` when `version_2012` is true, are read with IAM's
+/// variable syntax: the `${key, 'default'}` form, and a key name trimmed of
+/// the code points up to U+0020.
+///
+/// IAM reads that syntax only in a `2012-10-17` document and under the string
+/// and ARN operators, with or without a set qualifier or `IfExists` (IAM
+/// policy simulator, 2026-11-03, two samples per row). In a `2008-10-17`
+/// document and in one with no `Version`, `${aws:username, 'x'}` is compared
+/// as written under `StringEquals` and `StringNotEquals`. In a `2012-10-17`
+/// document a default does not take part under `NumericEquals`,
+/// `DateGreaterThan`, `IpAddress`, `Bool` or `Null`, while it does under
+/// `StringEquals` and `ArnLike`. Elsewhere the text between the braces is the
+/// key name as written, which is how condition values were read before the
+/// default form: `${key}` still expands there, as it did.
+fn reads_iam_variable_syntax(operator: &ConditionOperator, version_2012: bool) -> bool {
+    version_2012 && operator_reads_variables(operator)
+}
+
+/// Whether `operator`, unwrapped from its set qualifier and `IfExists`, is a
+/// string or ARN operator.
+fn operator_reads_variables(operator: &ConditionOperator) -> bool {
+    match operator {
+        ConditionOperator::ForAllValues(inner)
+        | ConditionOperator::ForAnyValue(inner)
+        | ConditionOperator::IfExists(inner) => operator_reads_variables(inner),
+        ConditionOperator::StringEquals
+        | ConditionOperator::StringNotEquals
+        | ConditionOperator::StringEqualsIgnoreCase
+        | ConditionOperator::StringNotEqualsIgnoreCase
+        | ConditionOperator::StringLike
+        | ConditionOperator::StringNotLike => true,
+        other => is_arn_operator(other),
+    }
 }
 
 /// Evaluate a statement's whole condition block (all conditions are ANDed).
 ///
 /// `NoMatch` dominates: if any condition is definitely not satisfied the
 /// statement does not apply, even if another condition is unresolvable. Failing
 /// that, any unresolvable condition makes the block `Unresolvable` (fail
-/// closed). Otherwise every condition matched.
+/// closed). Otherwise every condition matched. `version_2012` is whether the
+/// statement's document has `Version` `2012-10-17`, as in
+/// [`evaluate_condition_with`].
 pub fn evaluate_statement_conditions(
     conditions: &[Condition],
+    version_2012: bool,
     context: &impl ConditionContext,
 ) -> ConditionMatch {
     let mut any_unresolvable = false;
     for condition in conditions {
-        match evaluate_condition_match(condition, context) {
+        match evaluate_condition_match(condition, version_2012, context) {
             ConditionMatch::Match => {}
             ConditionMatch::NoMatch => return ConditionMatch::NoMatch,
             ConditionMatch::Unresolvable => any_unresolvable = true,
@@ -252,52 +311,29 @@ pub fn evaluate_statement_conditions(
 
 /// Expand IAM policy variables in a string value.
 ///
-/// Replaces `${variable}` patterns with their resolved values from the context.
-/// Supported variables: `aws:PrincipalTag/*` and any single-valued key the
-/// context can resolve. A variable that names an unresolvable key stays
-/// literal (IAM behavior), and so does one that names a multivalued key
-/// (`aws:TagKeys`, `dynamodb:LeadingKeys`, `dynamodb:Attributes`): a set of
-/// values has no one value to expand to. IAM leaves such a variable as text
-/// (measured 2026-10-27: a Deny on `aws:ResourceTag/team` equal to
-/// `${aws:TagKeys}` applies to no request, whatever order the request lists
-/// its tag keys in, and the same Deny under `StringNotEquals` applies to
-/// every request).
-fn expand_policy_variables(value: &str, context: &impl ConditionContext) -> String {
-    if !value.contains("${") {
-        return value.to_owned();
-    }
-
-    let mut result = String::with_capacity(value.len());
-    let mut rest = value;
-
-    while let Some(start) = rest.find("${") {
-        result.push_str(&rest[..start]);
-        let after_start = &rest[start + 2..];
-        if let Some(end) = after_start.find('}') {
-            let var_name = &after_start[..end];
-            let resolved = if context.is_multivalued_key(var_name) {
-                None
-            } else {
-                context.resolve_key(var_name)
-            };
-            if let Some(vals) = resolved {
-                // A single-valued key resolves to one value.
-                if let Some(v) = vals.first() {
-                    result.push_str(v);
-                }
-            } else {
-                // Unresolvable or multivalued: leave the variable literal.
-                result.push_str(&rest[start..start + 3 + end]);
-            }
-            rest = &after_start[end + 1..];
-        } else {
-            // No closing brace — copy literally.
-            result.push_str(&rest[start..]);
-            rest = "";
-        }
-    }
-    result.push_str(rest);
-    result
+/// Replaces `${variable}` and `${variable, 'default'}` patterns with their
+/// resolved values from the context (see [`super::variables`]). Supported
+/// variables: `aws:PrincipalTag/*` and any single-valued key the context can
+/// resolve. A variable whose key the request does not carry, with no default,
+/// stays literal, which gives IAM's answer except when the compared value
+/// holds the variable's text. A variable that names a multivalued key
+/// (`aws:TagKeys`, `dynamodb:LeadingKeys`, `dynamodb:Attributes`) also stays
+/// literal: a set of values has no one value to expand to. IAM leaves such a
+/// variable as text (measured 2026-10-27: a Deny on `aws:ResourceTag/team`
+/// equal to `${aws:TagKeys}` applies to no request, whatever order the
+/// request lists its tag keys in, and the same Deny under `StringNotEquals`
+/// applies to every request). A variable naming a key the context reports
+/// unresolvable also stays literal here, with or without a default; the
+/// condition fails closed on it before this expansion is compared
+/// ([`evaluate_condition_match`]). Without `iam_syntax`
+/// ([`reads_iam_variable_syntax`]) the default form is not read, so
+/// `${key, 'default'}` stays literal too.
+fn expand_policy_variables(
+    value: &str,
+    iam_syntax: bool,
+    context: &impl ConditionContext,
+) -> String {
+    super::variables::expand_condition_value(value, iam_syntax, context)
 }
 
 /// Unwrap an `IfExists` wrapper if present.

crates/auth/src/policy/condition/tests/three_case.rs

@@ -18,13 +18,19 @@ fn present_key_matches_or_not() {
         "aws:PrincipalArn",
         vec!["arn:a"],
     );
-    assert_eq!(evaluate_condition_match(&eq, &ctx), ConditionMatch::Match);
+    assert_eq!(
+        evaluate_condition_match(&eq, true, &ctx),
+        ConditionMatch::Match
+    );
     let ne = cond(
         ConditionOperator::StringEquals,
         "aws:PrincipalArn",
         vec!["arn:b"],
     );
-    assert_eq!(evaluate_condition_match(&ne, &ctx), ConditionMatch::NoMatch);
+    assert_eq!(
+        evaluate_condition_match(&ne, true, &ctx),
+        ConditionMatch::NoMatch
+    );
 }
 
 #[test]
@@ -38,15 +44,18 @@ fn absent_key_keeps_iam_semantics() {
         vec!["x"],
     );
     assert_eq!(
-        evaluate_condition_match(&pos, &ctx),
+        evaluate_condition_match(&pos, true, &ctx),
         ConditionMatch::NoMatch
     );
     let neg = cond(
         ConditionOperator::StringNotEquals,
         "aws:PrincipalTag/team",
         vec!["x"],
     );
-    assert_eq!(evaluate_condition_match(&neg, &ctx), ConditionMatch::Match);
+    assert_eq!(
+        evaluate_condition_match(&neg, true, &ctx),
+        ConditionMatch::Match
+    );
 }
 
 #[test]
@@ -62,12 +71,69 @@ fn unresolvable_key_is_unresolvable_for_every_operator() {
     ] {
         let c = cond(op, "aws:PrincipalOrgID", vec!["o-1"]);
         assert_eq!(
-            evaluate_condition_match(&c, &ctx),
+            evaluate_condition_match(&c, true, &ctx),
             ConditionMatch::Unresolvable
         );
     }
 }
 
+#[test]
+fn variable_naming_unresolvable_key_is_unresolvable_with_or_without_default() {
+    // Case 3 read through a condition value: the variable has no value to
+    // expand to, and the service, which has one, would not use the default.
+    // Every operator yields Unresolvable, so a Deny applies and an Allow does
+    // not grant whatever the operator says. A later value of the condition
+    // that names the key counts as the first one does.
+    let ctx = TestContext::new()
+        .with("aws:PrincipalTag/team", vec!["blue"])
+        .unresolvable("aws:PrincipalOrgID");
+    for values in [
+        vec!["${aws:PrincipalOrgID}"],
+        vec!["${aws:PrincipalOrgID, 'blue'}"],
+        vec!["${aws:PrincipalOrgID,'blue'}"],
+        vec!["x-${aws:PrincipalOrgID, 'blue'}-y"],
+        vec!["red", "${aws:PrincipalOrgID, 'blue'}"],
+    ] {
+        for op in [
+            ConditionOperator::StringEquals,
+            ConditionOperator::StringNotEquals,
+            ConditionOperator::StringLike,
+        ] {
+            let c = cond(op, "aws:PrincipalTag/team", values.clone());
+            assert_eq!(
+                evaluate_condition_match(&c, true, &ctx),
+                ConditionMatch::Unresolvable,
+                "{values:?}"
+            );
+        }
+    }
+    // The key present: its value is used, not the default.
+    let present = TestContext::new()
+        .with("aws:PrincipalTag/team", vec!["blue"])
+        .with("aws:PrincipalOrgID", vec!["blue"])
+        .unresolvable("aws:PrincipalOrgID");
+    let c = cond(
+        ConditionOperator::StringEquals,
+        "aws:PrincipalTag/team",
+        vec!["${aws:PrincipalOrgID, 'red'}"],
+    );
+    assert_eq!(
+        evaluate_condition_match(&c, true, &present),
+        ConditionMatch::Match
+    );
+    // The key absent, not unresolvable: the default is used.
+    let absent = TestContext::new().with("aws:PrincipalTag/team", vec!["blue"]);
+    let c = cond(
+        ConditionOperator::StringEquals,
+        "aws:PrincipalTag/team",
+        vec!["${aws:PrincipalOrgID, 'blue'}"],
+    );
+    assert_eq!(
+        evaluate_condition_match(&c, true, &absent),
+        ConditionMatch::Match
+    );
+}
+
 #[test]
 fn resolved_unresolvable_key_uses_its_value() {
     // If the context both lists a key as unresolvable and resolves it, the
@@ -80,7 +146,10 @@ fn resolved_unresolvable_key_uses_its_value() {
         "aws:PrincipalArn",
         vec!["arn:a"],
     );
-    assert_eq!(evaluate_condition_match(&c, &ctx), ConditionMatch::Match);
+    assert_eq!(
+        evaluate_condition_match(&c, true, &ctx),
+        ConditionMatch::Match
+    );
 }
 
 #[test]
@@ -103,7 +172,7 @@ fn statement_nomatch_dominates_unresolvable() {
         ),
     ];
     assert_eq!(
-        evaluate_statement_conditions(&conds, &ctx),
+        evaluate_statement_conditions(&conds, true, &ctx),
         ConditionMatch::NoMatch
     );
 }
@@ -126,7 +195,7 @@ fn statement_unresolvable_when_only_unresolvable_present() {
         ),
     ];
     assert_eq!(
-        evaluate_statement_conditions(&conds, &ctx),
+        evaluate_statement_conditions(&conds, true, &ctx),
         ConditionMatch::Unresolvable
     );
 }
@@ -140,12 +209,12 @@ fn statement_all_match() {
         vec!["contractors"],
     )];
     assert_eq!(
-        evaluate_statement_conditions(&conds, &ctx),
+        evaluate_statement_conditions(&conds, true, &ctx),
         ConditionMatch::Match
     );
     // An empty condition block always matches.
     assert_eq!(
-        evaluate_statement_conditions(&[], &ctx),
+        evaluate_statement_conditions(&[], true, &ctx),
         ConditionMatch::Match
     );
 }