feat(sts)!: fail closed on empty trust fields, check token type, require exp, log successes

crates/core/src/types.rs

@@ -158,8 +158,10 @@ pub struct RoleConfig {
     pub trusted_oidc_issuers: Vec<String>,
 
     /// Audience claim values accepted for this role. A token is accepted if its
-    /// `aud` claim matches any entry; empty (or absent/null) means no audience
-    /// restriction. Accepts a single string or a list, and the legacy
+    /// `aud` claim matches any entry. Empty (or absent/null) accepts **no**
+    /// token: the audience is what keeps a token minted for another service
+    /// from being exchanged here, so a role without one is misconfigured, not
+    /// open. Accepts a single string or a list, and the legacy
     /// `required_audience` key, for backward compatibility — set one key or the
     /// other, not both (specifying both is a config error).
     #[serde(
@@ -169,11 +171,21 @@ pub struct RoleConfig {
     )]
     pub required_audiences: Vec<String>,
 
-    /// Conditions on the subject claim (glob patterns).
-    /// e.g., "repo:myorg/myrepo:ref:refs/heads/main"
+    /// Conditions on the subject claim (glob patterns), e.g.
+    /// `"repo:myorg/myrepo:ref:refs/heads/main"`. A token's `sub` must match at
+    /// least one. Empty accepts **no** subject; to accept every subject, say
+    /// so with `"*"`.
     #[serde(default)]
     pub subject_conditions: Vec<String>,
 
+    /// Issuers whose tokens may omit `exp`, because the host tracks their
+    /// validity itself — its own long-lived API keys with server-side
+    /// revocation, say. Tokens from every other issuer must carry `exp`: a
+    /// third-party token with no expiry is an indefinitely replayable
+    /// credential its issuer never meant to issue.
+    #[serde(default)]
+    pub allow_missing_exp_from: Vec<String>,
+
     /// Buckets and prefixes this role can access.
     #[serde(default)]
     pub allowed_scopes: Vec<AccessScope>,
@@ -196,8 +208,8 @@ where
         Many(Vec<String>),
     }
     // `Option` so an explicit `null` (e.g. legacy `required_audience: null`)
-    // maps to "unrestricted", matching the old `Option<String>` behavior
-    // instead of failing to parse.
+    // parses as an empty list — which accepts no token — rather than failing
+    // to parse; config validation is what reports it.
     Ok(match Option::<OneOrMany>::deserialize(deserializer)? {
         None => vec![],
         Some(OneOrMany::One(s)) => vec![s],

crates/static-config/src/lib.rs

@@ -69,6 +69,18 @@ impl StaticConfig {
                     role.role_id
                 ));
             }
+            if role.required_audiences.is_empty() {
+                errors.push(format!(
+                    "role {:?} has no required_audiences (will never accept a token)",
+                    role.role_id
+                ));
+            }
+            if role.subject_conditions.is_empty() {
+                errors.push(format!(
+                    "role {:?} has no subject_conditions (will never accept a token; use \"*\" for any subject)",
+                    role.role_id
+                ));
+            }
         }
 
         // Check credentials
@@ -284,8 +296,9 @@ mod tests {
                 role_id: "my-role".into(),
                 name: "My Role".into(),
                 trusted_oidc_issuers: vec!["https://issuer.example.com".into()],
-                required_audiences: vec![],
-                subject_conditions: vec![],
+                required_audiences: vec!["my-audience".into()],
+                subject_conditions: vec!["*".into()],
+                allow_missing_exp_from: vec![],
                 allowed_scopes: vec![],
                 max_session_duration_secs: 3600,
             }],
@@ -358,6 +371,16 @@ mod tests {
         assert!(err.contains("no trusted_oidc_issuers"), "{}", err);
     }
 
+    #[test]
+    fn test_empty_audiences_and_subject_conditions_are_rejected() {
+        let mut config = valid_config();
+        config.roles[0].required_audiences.clear();
+        config.roles[0].subject_conditions.clear();
+        let err = config.validate().unwrap_err().to_string();
+        assert!(err.contains("no required_audiences"), "{}", err);
+        assert!(err.contains("no subject_conditions"), "{}", err);
+    }
+
     #[test]
     fn test_empty_access_key_id() {
         let mut config = valid_config();

crates/sts/README.md

@@ -28,6 +28,7 @@ Client signs S3 requests with temp creds
 Roles define who can assume them:
 
 - **`trusted_oidc_issuers`** — accepted OIDC providers (e.g., `https://token.actions.githubusercontent.com`)
-- **`required_audiences`** — accepted `aud` claim values (string or list); a token passes if its `aud` matches any. Empty/omitted means unrestricted. Legacy `required_audience` (single string) still accepted.
-- **`subject_conditions`** — glob patterns for the `sub` claim (e.g., `repo:myorg/*`)
+- **`required_audiences`** — accepted `aud` claim values (string or list); a token passes if its `aud` matches any. Empty/omitted accepts no token. Legacy `required_audience` (single string) still accepted.
+- **`subject_conditions`** — glob patterns for the `sub` claim (e.g., `repo:myorg/*`). Empty accepts no subject; `"*"` accepts any.
+- **`allow_missing_exp_from`** — issuers whose tokens may omit `exp` because the host tracks their validity; every other issuer's tokens must carry it
 - **`allowed_scopes`** — buckets, prefixes, and actions the minted credentials grant

crates/sts/src/jwks.rs

@@ -122,12 +122,12 @@ fn rsa_public_key_from_components(n: &str, e: &str) -> Result<RsaPublicKey, Prox
 
 /// Whether a token's `aud` claim is acceptable for a set of accepted audiences.
 ///
-/// An empty `accepted` set means no audience restriction (always allowed). The
-/// `aud` claim may be a single string or an array; it passes if any of its
-/// values is in `accepted`.
+/// An empty `accepted` set accepts nothing: a role with no audience would take
+/// a token minted for any other service. The `aud` claim may be a single
+/// string or an array; it passes if any of its values is in `accepted`.
 fn audience_allowed(aud_claim: Option<&serde_json::Value>, accepted: &[String]) -> bool {
     if accepted.is_empty() {
-        return true;
+        return false;
     }
     match aud_claim {
         Some(serde_json::Value::String(aud)) => accepted.iter().any(|a| a == aud),
@@ -184,10 +184,40 @@ pub fn verify_token(
             ProxyError::InvalidOidcToken(format!("JWT signature verification failed: {}", e))
         })?;
 
-    // Decode and validate claims
     let claims = decode_jwt_segment(payload_b64)?;
+    validate_claims(
+        &header,
+        &claims,
+        issuer,
+        role,
+        chrono::Utc::now().timestamp(),
+    )?;
+    Ok(claims)
+}
+
+/// Validate a signature-verified token's header and claims against `role`.
+///
+/// Kept apart from signature verification so every rule here is testable
+/// without a key pair; `verify_token` calls it once the signature checks out.
+fn validate_claims(
+    header: &serde_json::Value,
+    claims: &serde_json::Value,
+    issuer: &str,
+    role: &RoleConfig,
+    now: i64,
+) -> Result<(), ProxyError> {
+    // A token that says what it is must say it is a JWT. Access tokens
+    // (`at+jwt`) and other typed tokens are not identity tokens, whoever
+    // signed them.
+    if let Some(typ) = header.get("typ").and_then(|v| v.as_str()) {
+        if !typ.eq_ignore_ascii_case("JWT") {
+            return Err(ProxyError::InvalidOidcToken(format!(
+                "unsupported token type: {}",
+                typ
+            )));
+        }
+    }
 
-    // Validate issuer
     let token_issuer = claims.get("iss").and_then(|v| v.as_str()).unwrap_or("");
     if token_issuer != issuer {
         return Err(ProxyError::InvalidOidcToken(format!(
@@ -196,21 +226,28 @@ pub fn verify_token(
         )));
     }
 
-    // Validate audience if restricted.
     if !audience_allowed(claims.get("aud"), &role.required_audiences) {
         return Err(ProxyError::InvalidOidcToken(format!(
             "audience mismatch: expected one of {:?}",
             role.required_audiences
         )));
     }
 
-    // Validate time-based claims with clock skew tolerance
-    let now = chrono::Utc::now().timestamp();
     const CLOCK_SKEW_SECS: i64 = 60;
 
-    if let Some(exp) = claims.get("exp").and_then(|v| v.as_i64()) {
-        if now > exp + CLOCK_SKEW_SECS {
-            return Err(ProxyError::InvalidOidcToken("token has expired".into()));
+    match claims.get("exp").and_then(|v| v.as_i64()) {
+        Some(exp) => {
+            if now > exp + CLOCK_SKEW_SECS {
+                return Err(ProxyError::InvalidOidcToken("token has expired".into()));
+            }
+        }
+        // Only an issuer the host vouches for may leave expiry to the host.
+        None => {
+            if !role.allow_missing_exp_from.iter().any(|i| i == issuer) {
+                return Err(ProxyError::InvalidOidcToken(
+                    "token has no exp claim".into(),
+                ));
+            }
         }
     }
 
@@ -222,7 +259,7 @@ pub fn verify_token(
         }
     }
 
-    Ok(claims)
+    Ok(())
 }
 
 /// In-memory cache for JWKS responses, keyed by issuer URL.
@@ -319,13 +356,85 @@ impl JwksCache {
 
 #[cfg(test)]
 mod tests {
-    use super::audience_allowed;
+    use super::{audience_allowed, validate_claims};
+    use multistore::types::RoleConfig;
     use serde_json::json;
 
+    const ISSUER: &str = "https://issuer.example";
+    const NOW: i64 = 1_700_000_000;
+
+    fn role() -> RoleConfig {
+        RoleConfig {
+            role_id: "r".into(),
+            name: "r".into(),
+            trusted_oidc_issuers: vec![ISSUER.into()],
+            required_audiences: vec!["aud".into()],
+            subject_conditions: vec!["*".into()],
+            allow_missing_exp_from: vec![],
+            allowed_scopes: vec![],
+            max_session_duration_secs: 3600,
+        }
+    }
+
+    fn claims() -> serde_json::Value {
+        json!({"iss": ISSUER, "aud": "aud", "sub": "s", "exp": NOW + 300})
+    }
+
+    #[test]
+    fn empty_accepted_denies() {
+        assert!(!audience_allowed(None, &[]));
+        assert!(!audience_allowed(Some(&json!("anything")), &[]));
+    }
+
+    #[test]
+    fn a_well_formed_token_passes() {
+        validate_claims(&json!({"typ": "JWT"}), &claims(), ISSUER, &role(), NOW).unwrap();
+        validate_claims(&json!({}), &claims(), ISSUER, &role(), NOW).unwrap();
+    }
+
+    #[test]
+    fn a_typed_non_jwt_is_rejected() {
+        let err = validate_claims(&json!({"typ": "at+jwt"}), &claims(), ISSUER, &role(), NOW)
+            .unwrap_err()
+            .to_string();
+        assert!(err.contains("unsupported token type"), "{}", err);
+    }
+
+    #[test]
+    fn a_role_with_no_audience_accepts_nothing() {
+        let mut role = role();
+        role.required_audiences.clear();
+        let err = validate_claims(&json!({}), &claims(), ISSUER, &role, NOW)
+            .unwrap_err()
+            .to_string();
+        assert!(err.contains("audience mismatch"), "{}", err);
+    }
+
+    #[test]
+    fn exp_is_required_unless_the_issuer_is_exempt() {
+        let mut claims = claims();
+        claims.as_object_mut().unwrap().remove("exp");
+
+        let err = validate_claims(&json!({}), &claims, ISSUER, &role(), NOW)
+            .unwrap_err()
+            .to_string();
+        assert!(err.contains("no exp claim"), "{}", err);
+
+        let mut exempt = role();
+        exempt.allow_missing_exp_from = vec![ISSUER.into()];
+        validate_claims(&json!({}), &claims, ISSUER, &exempt, NOW).unwrap();
+    }
+
     #[test]
-    fn empty_accepted_means_no_restriction() {
-        assert!(audience_allowed(None, &[]));
-        assert!(audience_allowed(Some(&json!("anything")), &[]));
+    fn an_expired_token_is_rejected_beyond_the_skew() {
+        let mut claims = claims();
+        claims["exp"] = json!(NOW - 30);
+        validate_claims(&json!({}), &claims, ISSUER, &role(), NOW).unwrap();
+        claims["exp"] = json!(NOW - 61);
+        let err = validate_claims(&json!({}), &claims, ISSUER, &role(), NOW)
+            .unwrap_err()
+            .to_string();
+        assert!(err.contains("expired"), "{}", err);
     }
 
     #[test]

crates/sts/src/lib.rs

@@ -155,20 +155,12 @@ pub async fn assume_role_with_web_identity<C: CredentialRegistry>(
     let key = jwks::find_key(&jwks, kid)?;
     let claims = jwks::verify_token(&sts_request.web_identity_token, key, issuer, &role)?;
 
-    // Check subject conditions
     let subject = claims.get("sub").and_then(|v| v.as_str()).unwrap_or("");
-
-    if !role.subject_conditions.is_empty() {
-        let matches = role
-            .subject_conditions
-            .iter()
-            .any(|pattern| subject_matches(subject, pattern));
-        if !matches {
-            return Err(ProxyError::InvalidOidcToken(format!(
-                "subject '{}' does not match any conditions",
-                subject
-            )));
-        }
+    if !subject_allowed(subject, &role.subject_conditions) {
+        return Err(ProxyError::InvalidOidcToken(format!(
+            "subject '{}' does not match any conditions",
+            subject
+        )));
     }
 
     // Mint temporary credentials (AWS enforces 900s minimum)
@@ -178,14 +170,31 @@ pub async fn assume_role_with_web_identity<C: CredentialRegistry>(
         .unwrap_or(3600)
         .clamp(MIN_SESSION_DURATION_SECS, role.max_session_duration_secs);
 
-    let mut creds = sts::mint_temporary_credentials(&role, subject, duration, key_prefix, &claims);
+    let mut creds = sts::mint_temporary_credentials(&role, subject, duration, key_prefix, &claims)?;
 
     // Encrypt the full credentials into the session token — stateless, no storage needed
     creds.session_token = token_key.seal(&creds)?;
 
+    tracing::info!(
+        issuer,
+        subject,
+        role = %role.role_id,
+        duration_secs = duration,
+        "STS exchange succeeded"
+    );
+
     Ok(creds)
 }
 
+/// Whether `subject` matches at least one of `conditions`. An empty list
+/// matches nothing: "any subject" has to be said, with `"*"`, so that a role
+/// which forgot its conditions fails closed rather than open.
+fn subject_allowed(subject: &str, conditions: &[String]) -> bool {
+    conditions
+        .iter()
+        .any(|pattern| subject_matches(subject, pattern))
+}
+
 /// Simple glob-style matching for subject conditions.
 /// Supports `*` as a wildcard for any sequence of characters.
 fn subject_matches(subject: &str, pattern: &str) -> bool {
@@ -232,6 +241,16 @@ fn subject_matches(subject: &str, pattern: &str) -> bool {
 mod tests {
     use super::*;
 
+    #[test]
+    fn no_subject_conditions_means_no_subject_is_allowed() {
+        assert!(!subject_allowed("repo:org/repo:ref:refs/heads/main", &[]));
+        assert!(subject_allowed("anything", &["*".to_string()]));
+        assert!(subject_allowed(
+            "repo:org/repo:ref:refs/heads/main",
+            &["repo:other/*".to_string(), "repo:org/*".to_string()]
+        ));
+    }
+
     #[test]
     fn test_subject_matching() {
         // Trailing wildcard