fix(sts): escape caller text in exchange error bodies

README.md

@@ -111,7 +111,7 @@ Set in `wrangler.toml` or via the Cloudflare dashboard:
 | `SOURCE_API_URL`             | `https://source.coop`       | Source Cooperative API base URL                                                                                                    |
 | `LOG_LEVEL`                  | `WARN`                      | Tracing level (`TRACE`, `DEBUG`, `INFO`, `WARN`, `ERROR`)                                                                          |
 | `AUTH_ISSUER`                | `https://auth.source.coop`  | The person issuer trusted for `/.sts` token exchange; its tokens act as their own subject                                          |
-| `AUTH_AUDIENCE`              | —                           | Comma-separated OAuth client ID(s) that `/.sts` subject tokens must be issued to (`aud` claim); a token is accepted if it matches any. Unset = `/.sts` token exchange is disabled (returns 501) |
+| `AUTH_AUDIENCE`              | —                           | Comma-separated OAuth client ID(s) that `/.sts` subject tokens must be issued to (`aud` claim); a token is accepted if it matches any. Unset = person-token exchange at `/.sts` is disabled (returns 501); API keys and platform tokens still exchange |
 | `PLATFORM_ISSUERS`           | —                           | JSON object from each platform issuer URL to the audiences its tokens must carry, such as `{"https://token.actions.githubusercontent.com": ["https://data.source.coop"]}`. An issuer with no audience is refused. Unset = no platform issuer is trusted |
 | `OIDC_PROVIDER_ISSUER`       | `https://data.source.coop`  | Issuer URL for minted JWTs and OIDC discovery                                                                                      |
 | `OIDC_PROVIDER_KID`          | `data-proxy-1`              | Key ID for the active signing key                                                                                                  |

src/config.rs

@@ -97,16 +97,23 @@ fn build_config(env: &Env) -> AppConfig {
     if auth_audiences.is_empty() {
         // Fail closed: without an audience restriction, an ID token minted for
         // ANY OAuth client of AUTH_ISSUER could be exchanged for a user's
-        // credentials, so /.sts is disabled entirely (returns 501) until set.
-        tracing::warn!("AUTH_AUDIENCE not set: /.sts token exchange is disabled (returns 501)");
+        // credentials, so its exchange is disabled (returns 501) until set.
+        tracing::warn!(
+            "AUTH_AUDIENCE not set: person-token exchange at /.sts is disabled (returns 501)"
+        );
     }
 
     // Platform identity providers (GitHub Actions, say), each with its own
     // audiences. Unset trusts none.
-    let platform_issuers = env
+    let mut platform_issuers = env
         .var("PLATFORM_ISSUERS")
         .map(|v| crate::platform::parse_issuers(&v.to_string()))
         .unwrap_or_default();
+    // The platform path claims its issuers' tokens ahead of the STS route, so
+    // an entry for the person issuer would refuse every person exchange.
+    if platform_issuers.remove(&auth_issuer).is_some() {
+        tracing::error!(issuer = %auth_issuer, "PLATFORM_ISSUERS names AUTH_ISSUER; ignoring that entry");
+    }
 
     // Ceiling for client-requested DurationSeconds on /.sts. Unset → 3600 (1h),
     // matching multistore's own default so behavior is unchanged until raised.

src/lib.rs

@@ -40,7 +40,7 @@ use multistore_oidc_provider::{HttpExchange, OidcCredentialProvider, OidcProvide
 use multistore_path_mapping::{MappedRegistry, PathMapping};
 use multistore_sts::jwks::JwksCache;
 use multistore_sts::route_handler::StsRouterExt;
-use multistore_sts::{build_sts_error_response, build_sts_response, try_parse_sts_request};
+use multistore_sts::{build_sts_response, try_parse_sts_request};
 use object_path::{extract_path_segments, is_keyless_write, mapped_copy_source};
 use std::sync::OnceLock;
 use sts::StsCredentialRegistry;
@@ -171,23 +171,6 @@ async fn fetch(req: web_sys::Request, env: Env, ctx: Context) -> Result<web_sys:
     // writable and signable) and the backend-auth middleware signs them. See
     // `authz` and `backend_auth`.
 
-    // ── Short-circuit: STS disabled (fail closed) ───────────────────
-    // `/.sts` requires an audience restriction (AUTH_AUDIENCE) to be safe —
-    // without it, an ID token minted for any OAuth client of AUTH_ISSUER could
-    // be exchanged for a user's credentials. When unset, refuse the endpoint
-    // with a 501 rather than serving it unrestricted.
-    if parts.path == "/.sts" && config.auth_audiences.is_empty() {
-        let resp = ErrorResponse {
-            code: "NotImplemented".to_string(),
-            message: "STS token exchange is not configured".to_string(),
-            resource: String::new(),
-            request_id: request_id.clone(),
-        };
-        return Ok(add_cors(
-            GatewayResponse::Response(ProxyResult::xml(501, resp.to_xml())).into_web_sys(),
-        ));
-    }
-
     // ── Short-circuit: write to a keyless path ──────────────────────
     // A keyless PUT/DELETE (e.g. `aws s3 cp f s3://account/product` with no
     // trailing slash) targets the product root, which has no object key.
@@ -253,6 +236,24 @@ async fn fetch(req: web_sys::Request, env: Env, ctx: Context) -> Result<web_sys:
         }
     }
 
+    // ── Short-circuit: STS disabled (fail closed) ───────────────────
+    // The person issuer's route requires an audience restriction
+    // (AUTH_AUDIENCE) to be safe — without it, an ID token minted for any
+    // OAuth client of AUTH_ISSUER could be exchanged for a user's credentials.
+    // When unset, refuse it with a 501 rather than serving it unrestricted.
+    // API keys and platform tokens, answered above, do not depend on it.
+    if parts.path == "/.sts" && config.auth_audiences.is_empty() {
+        let resp = ErrorResponse {
+            code: "NotImplemented".to_string(),
+            message: "STS token exchange is not configured".to_string(),
+            resource: String::new(),
+            request_id: request_id.clone(),
+        };
+        return Ok(add_cors(
+            GatewayResponse::Response(ProxyResult::xml(501, resp.to_xml())).into_web_sys(),
+        ));
+    }
+
     // ── Build gateway with route handlers ──────────────────────────
     let registry = SourceCoopRegistry::new(
         config.api_base_url.clone(),
@@ -551,17 +552,35 @@ async fn api_key_exchange(
             // API being unreachable, which fails closed as a 500 the SDK retries.
             Err(e) => {
                 tracing::warn!(%request_id, error = %e, "API key exchange failed");
-                build_sts_error_response(&e)
+                sts_refusal(&e, request_id)
             }
         },
     )
 }
 
 /// `InvalidIdentityToken`, with the request id in the message.
 fn key_refusal(message: &str, request_id: &str) -> (u16, String) {
-    build_sts_error_response(&ProxyError::InvalidOidcToken(with_request_id(
-        message, request_id,
-    )))
+    sts_refusal(&ProxyError::InvalidOidcToken(message.into()), request_id)
+}
+
+/// The STS error for `e`, mapped as multistore's `build_sts_error_response`
+/// maps it, with the request id in the message. Built here because that one
+/// writes the message unescaped, and it can carry a caller's `RoleArn` or a
+/// token's `kid`.
+fn sts_refusal(e: &ProxyError, request_id: &str) -> (u16, String) {
+    let (status, code, message) = match e {
+        ProxyError::RoleNotFound(r) => (
+            400,
+            "MalformedPolicyDocument",
+            format!("role not found: {r}"),
+        ),
+        ProxyError::InvalidOidcToken(m) => (400, "InvalidIdentityToken", m.clone()),
+        ProxyError::InvalidRequest(m) => (400, "InvalidParameterValue", m.clone()),
+        ProxyError::AccessDenied => (403, "AccessDenied", "access denied".to_string()),
+        _ => (500, "InternalError", "internal error".to_string()),
+    };
+    let message = with_request_id(&message, request_id);
+    (status, sts_error_xml(code, &message))
 }
 
 /// `message` with the request id, if there is one: SDKs show a user the
@@ -668,9 +687,13 @@ fn throttled() -> (u16, String) {
     )
 }
 
-/// An STS-shaped error body with a code or message `build_sts_error_response`
-/// does not produce.
+/// An STS-shaped error body. The message is escaped: it can carry text the
+/// caller sent, and the body is served as XML.
 fn sts_error_xml(code: &str, message: &str) -> String {
+    let message = message
+        .replace('&', "&amp;")
+        .replace('<', "&lt;")
+        .replace('>', "&gt;");
     format!(
         "<?xml version=\"1.0\" encoding=\"UTF-8\"?>\n<ErrorResponse><Error><Code>{code}</Code><Message>{message}</Message></Error></ErrorResponse>"
     )
@@ -688,9 +711,12 @@ async fn platform_exchange(
     api_auth: &ApiAuth,
     request_id: &str,
 ) -> Option<(u16, String)> {
-    let sts = try_parse_sts_request(parts.query.as_deref())
+    let mut sts = try_parse_sts_request(parts.query.as_deref())
         .or_else(|| try_parse_sts_request(parts.form_body.as_deref()))?
         .ok()?;
+    // SDKs send a token file's contents as-is, and `jq -r … > file` ends it in
+    // a newline, which the signature segment's base64 decode rejects.
+    sts.web_identity_token = sts.web_identity_token.trim().to_string();
     let (header, claims) = platform::unverified(&sts.web_identity_token)?;
     let issuer = claims.get("iss")?.as_str()?;
     let audiences = config.platform_issuers.get(issuer)?;
@@ -740,7 +766,7 @@ async fn exchange_platform_token(
     } = token;
     let failed = |e: ProxyError| {
         tracing::warn!(%request_id, %issuer, error = %e, "platform token exchange failed");
-        build_sts_error_response(&e)
+        sts_refusal(&e, request_id)
     };
     let not_authorized = || {
         let message = "Not authorized to perform sts:AssumeRoleWithWebIdentity";
@@ -757,7 +783,6 @@ async fn exchange_platform_token(
         config.sts_max_session_duration_secs,
     )
     .ok_or_else(|| failed(ProxyError::RoleNotFound(sts.role_arn.clone())))?;
-    // No angle brackets in the message: the STS error body carries it unescaped.
     let account = sts::account(&sts.role_arn).ok_or_else(|| {
         failed(ProxyError::InvalidRequest(
             "RoleArn must name the account to act as: arn:aws:iam::ACCOUNT:role/ROLE".into(),

src/source_api/cache.rs

@@ -57,10 +57,9 @@ const KEY_STANDING_CACHE_SECS: u32 = 60; // 1 minute
 /// reason: a trust that is removed should stop minting quickly (ADR-014).
 const TRUST_CACHE_SECS: u32 = 60; // 1 minute
 
-/// A refusal from the trusts route, cached under its own key: long enough that
-/// replaying one token its account does not trust costs about one lookup per
-/// 10 seconds, however many addresses it comes from, and short enough that a
-/// trust just added works within seconds.
+/// A refusal from the trusts route: long enough that replaying one token its
+/// account does not trust costs about one lookup per 10 seconds per data
+/// center, and short enough that a trust just added works within seconds.
 const REFUSED_TRUST_CACHE_SECS: u32 = 10;
 
 // ── Public cache functions ─────────────────────────────────────────
@@ -220,7 +219,7 @@ pub async fn get_or_fetch_key_standing(
 /// assume-role call. Asked as the account itself. The route says yes with a
 /// 200, cached for `TRUST_CACHE_SECS` like every 200, and no with a 403 (a 401
 /// for an account it cannot resolve), which `cached_fetch` leaves uncached and
-/// this caches for `REFUSED_TRUST_CACHE_SECS` under a key of its own.
+/// this caches as `{"trusted":false}` for `REFUSED_TRUST_CACHE_SECS`.
 pub async fn get_or_fetch_trust(
     api_base_url: &str,
     account: &str,
@@ -241,11 +240,6 @@ pub async fn get_or_fetch_trust(
         utf8_percent_encode(issuer, PATH_SEGMENT),
         utf8_percent_encode(subject, PATH_SEGMENT),
     );
-    let refused_key = format!("{cache_key}&refused");
-    let cache = worker::Cache::default();
-    if matches!(cache.get(&refused_key, false).await, Ok(Some(_))) {
-        return Err(ProxyError::AccessDenied);
-    }
     let body = serde_json::json!({ "issuer": issuer, "subject": subject }).to_string();
     let answer = cached_fetch::<TrustAnswer>(
         &cache_key,
@@ -260,17 +254,21 @@ pub async fn get_or_fetch_trust(
     .await;
     let trusted = match answer {
         Ok(answer) => answer.trusted,
-        Err(ProxyError::AccessDenied) => false,
+        Err(ProxyError::AccessDenied) => {
+            let cache = worker::Cache::default();
+            let refused = r#"{"trusted":false}"#;
+            cache_put(&cache, &cache_key, refused, REFUSED_TRUST_CACHE_SECS).await;
+            false
+        }
         Err(e) => return Err(e),
     };
-    if !trusted {
-        // A 200 saying no was cached like any 200: drop it, so a no is held
-        // for `REFUSED_TRUST_CACHE_SECS` whichever way the route said it.
-        let _ = cache.delete(cache_key.as_str(), false).await;
-        cache_put(&cache, &refused_key, "{}", REFUSED_TRUST_CACHE_SECS).await;
-        return Err(ProxyError::AccessDenied);
+    // ponytail: a 200 saying no (which the route never sends) is held for
+    // TRUST_CACHE_SECS like any 200; still refused, only slower to flip to yes.
+    if trusted {
+        Ok(())
+    } else {
+        Err(ProxyError::AccessDenied)
     }
-    Ok(())
 }
 
 /// The trusts route's answer. Its status already says yes (200) or no (403);

tests/test_platform_trust.py

@@ -63,6 +63,15 @@ def test_a_platform_token_must_name_the_account_it_acts_as():
     assert "RoleArn must name the account" in sts_fields(resp)["Message"]
 
 
+def test_a_refusal_escapes_what_the_caller_sent():
+    """RoleArn is echoed in the message; markup in it stays text."""
+    role_arn = 'arn:aws:iam::ab--cd:role/<x:script xmlns:x="http://www.w3.org/1999/xhtml">&'
+    resp = exchange(forged(), role_arn)
+    assert resp.status_code == 400
+    assert "<x:script" not in resp.text
+    assert role_arn in sts_fields(resp)["Message"]
+
+
 @pytest.mark.parametrize(
     "account",
     ["alice", "2c5b4f0e-8a3b-4e2d-9a1f-3c4d5e6f7a8b", "000000000000"],
@@ -83,7 +92,9 @@ def test_a_forged_token_is_refused_before_any_trust_lookup():
     before = trust_lookups(TRUST_ACCOUNT)
     resp = exchange(forged(), as_account(TRUST_ACCOUNT))
     assert resp.status_code == 400
-    assert sts_fields(resp)["Code"] == "InvalidIdentityToken"
+    fields = sts_fields(resp)
+    assert fields["Code"] == "InvalidIdentityToken"
+    assert fields["Message"].endswith(f"(request id {RAY})")
     assert trust_lookups(TRUST_ACCOUNT) == before
 
 
@@ -115,6 +126,13 @@ def test_a_trusted_workflow_gets_credentials_that_act_as_the_account():
     assert subjects[f"/api/v1/products/{WRITE_ACCOUNT}/{product}"] == TRUST_ACCOUNT
 
 
+@needs_token
+def test_a_token_read_from_a_file_may_end_in_a_newline():
+    """SDKs send AWS_WEB_IDENTITY_TOKEN_FILE's contents untrimmed."""
+    resp = exchange(ID_TOKEN + "\n", as_account(TRUST_ACCOUNT))
+    assert resp.status_code == 200, resp.text[:300]
+
+
 @needs_token
 def test_an_account_that_does_not_trust_the_workflow_refuses_it():
     untrusting = as_account("ci-tests--someone-else")

wrangler.preview.toml

@@ -18,7 +18,7 @@ OIDC_PROVIDER_KID = "data-proxy-1"
 
 # Preview is staging-equivalent (shares staging analytics + log stream), so
 # trust the staging Ory issuer and accept the staging frontend + CLI client_ids.
-# AUTH_AUDIENCE must be set or /.sts fail-closes with 501.
+# AUTH_AUDIENCE must be set or person-token exchange at /.sts fail-closes with 501.
 AUTH_ISSUER = "https://auth.staging.source.coop"
 AUTH_AUDIENCE = "1123dfa8-469f-44fe-b9f4-9b76f06fd325,a79c9537-be78-454a-9ea1-b96a1be811cc"  # staging frontend + source-coop-cli client_ids
 # Staging's platform issuers too: a GitHub Actions workflow calling a preview

wrangler.toml

@@ -45,7 +45,7 @@ STS_MAX_SESSION_DURATION_SECS = "43200"
 # Required vars (to enable /.sts token exchange):
 #   AUTH_AUDIENCE - comma-separated OAuth client_id(s) that /.sts subject tokens
 #                   must be issued to (the `aud` claim). A token is accepted if
-#                   it matches any. Unset = /.sts is disabled (returns 501).
+#                   it matches any. Unset = person-token exchange at /.sts is disabled (returns 501).
 # Optional vars:
 #   PLATFORM_ISSUERS - JSON object from platform issuer URL to the audiences its
 #                      tokens must carry. Unset = no platform issuer is trusted.