From 62639db018f55fba95ab8ee1b0bd46e50d2515f7 Mon Sep 17 00:00:00 2001 From: Russell Bryant Date: Tue, 30 Jun 2026 14:21:52 -0400 Subject: [PATCH 01/16] feat(providers): add AWS STS credential refresh and aws-s3 provider profile MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Add server-side AWS STS AssumeRole credential refresh so gateways can mint short-lived AWS credentials for sandboxes. Combined with the SigV4 proxy re-signing from #1638, sandboxes can access S3 (and other AWS services) without ever seeing real credentials. - New `AwsStsAssumeRole` variant in `ProviderCredentialRefreshStrategy`. - `provider_refresh.rs` implements the refresh loop: calls `sts:AssumeRole` using the gateway's ambient AWS credentials (or explicit long-lived keys from material), stores the resulting `AccessKeyId`, `SecretAccessKey`, and `SessionToken` on the provider, and schedules re-rotation before expiry. - `ConfigureProviderRefresh` and `RotateProviderCredential` RPCs gated behind the `providers_v2_enabled` setting. Validates that configuring STS refresh for one provider won't collide with credential keys already held by another provider attached to the same sandbox. - Proto: `AWS_STS_ASSUME_ROLE = 3` on `ProviderCredentialRefreshStrategy`. AWS provider profiles follow the same pattern as the existing `aws-bedrock` profile: a generic base plus service-specific variants. - `providers/aws.yaml`: base AWS profile with STS refresh material (role_arn, session_name, external_id, aws_region, optional long-lived keys) but no endpoints or binaries allowlist. Intended for AWS services that don't yet have a dedicated profile — the user attaches their own policy to supply endpoints. This is the same role `google-cloud` plays relative to `google-vertex-ai`. - `providers/aws-s3.yaml`: S3-specific profile that adds pre-configured endpoints covering regional (`*.s3.*.amazonaws.com`), global (`*.s3.amazonaws.com` with `signing_region: us-east-1`), and dualstack variants. All endpoints use `credential_signing: sigv4` and `signing_service: s3` so the proxy re-signs requests automatically. Includes a binaries allowlist for Python, curl, and the AWS CLI. - End-to-end walkthrough added in `examples/aws-s3-sts.md`. - Provider docs updated in `docs/sandboxes/manage-providers.mdx`. - `validate_host_wildcard` in `openshell-policy` now permits `*` as a complete middle label (e.g. `*.s3.*.amazonaws.com`) in addition to the existing leading-label and intra-label positions. A bare `*` label matches exactly one DNS label via Rego's `glob.match` with `.` separator, consistent with TLS wildcard semantics. - OPA endpoint matching and L7 config lookup updated to handle middle-label wildcards: `host_matches_wildcard_middle` rule and `endpoint_config_for_middle_wildcard` in Rego. - Validation rejects `**` in any position and partial wildcards in middle labels (e.g. `s*.example.com` in a non-leading label). - Allow bodyless requests (GET/HEAD/DELETE) through the SignBody path. boto3 sets `x-amz-content-sha256` to the empty-body hash on all requests, routing them through SignBody via `detect_payload_mode`. Previously `BodyLength::None` was rejected; now it signs with the empty-body hash, matching AWS SDK behavior. - Added `AwsStsAssumeRole` match arm to the refresh strategy label in the provider settings view. Validated end-to-end: gateway mints STS credentials, sandbox runs boto3 PutObject/ListObjects/GetObject against real S3 through the SigV4 re-signing proxy. The sandbox sees only placeholder credentials. Signed-off-by: Russell Bryant --- Cargo.lock | 435 +++++++++++++++--- Cargo.toml | 4 + architecture/security-policy.md | 11 +- crates/openshell-cli/src/main.rs | 2 + crates/openshell-cli/src/run.rs | 2 + crates/openshell-policy/src/lib.rs | 103 +++++ crates/openshell-providers/src/profiles.rs | 92 ++++ crates/openshell-server/Cargo.toml | 2 + crates/openshell-server/src/grpc/policy.rs | 2 +- crates/openshell-server/src/grpc/provider.rs | 248 ++++++++++ .../openshell-server/src/provider_refresh.rs | 328 +++++++++++++ .../src/l7/mod.rs | 74 ++- .../src/l7/rest.rs | 97 +++- .../openshell-supervisor-network/src/opa.rs | 82 ++++ crates/openshell-tui/src/app.rs | 3 + docs/sandboxes/manage-providers.mdx | 46 +- examples/aws-s3-sts.md | 269 +++++++++++ proto/openshell.proto | 1 + providers/aws-s3.yaml | 112 +++++ providers/aws.yaml | 49 ++ 20 files changed, 1886 insertions(+), 76 deletions(-) create mode 100644 examples/aws-s3-sts.md create mode 100644 providers/aws-s3.yaml create mode 100644 providers/aws.yaml diff --git a/Cargo.lock b/Cargo.lock index 76dedf0625..462848d189 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -267,6 +267,31 @@ dependencies = [ "cc", ] +[[package]] +name = "aws-config" +version = "1.8.15" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "11493b0bad143270fb8ad284a096dd529ba91924c5409adeac856cc1bf047dbc" +dependencies = [ + "aws-credential-types", + "aws-runtime", + "aws-sdk-sts", + "aws-smithy-async", + "aws-smithy-http", + "aws-smithy-json", + "aws-smithy-runtime", + "aws-smithy-runtime-api", + "aws-smithy-types", + "aws-types", + "bytes", + "fastrand", + "http 1.4.0", + "time", + "tokio", + "tracing", + "url", +] + [[package]] name = "aws-credential-types" version = "1.2.14" @@ -302,6 +327,56 @@ dependencies = [ "fs_extra", ] +[[package]] +name = "aws-runtime" +version = "1.7.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "5fc0651c57e384202e47153c1260b84a9936e19803d747615edf199dc3b98d17" +dependencies = [ + "aws-credential-types", + "aws-sigv4", + "aws-smithy-async", + "aws-smithy-http", + "aws-smithy-runtime", + "aws-smithy-runtime-api", + "aws-smithy-types", + "aws-types", + "bytes", + "bytes-utils", + "fastrand", + "http 1.4.0", + "http-body 1.0.1", + "percent-encoding", + "pin-project-lite", + "tracing", + "uuid", +] + +[[package]] +name = "aws-sdk-sts" +version = "1.102.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "0fc35b7a14cabdad13795fbbbd26d5ddec0882c01492ceedf2af575aad5f37dd" +dependencies = [ + "aws-credential-types", + "aws-runtime", + "aws-smithy-async", + "aws-smithy-http", + "aws-smithy-json", + "aws-smithy-observability", + "aws-smithy-query", + "aws-smithy-runtime", + "aws-smithy-runtime-api", + "aws-smithy-types", + "aws-smithy-xml", + "aws-types", + "fastrand", + "http 0.2.12", + "http 1.4.0", + "regex-lite", + "tracing", +] + [[package]] name = "aws-sigv4" version = "1.4.2" @@ -356,6 +431,92 @@ dependencies = [ "tracing", ] +[[package]] +name = "aws-smithy-http-client" +version = "1.1.13" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "5c3ef8931ad1c98aa6a55b4256f847f3116090819844e0dd41ea682cac5dd2d3" +dependencies = [ + "aws-smithy-async", + "aws-smithy-runtime-api", + "aws-smithy-types", + "h2 0.3.27", + "h2 0.4.13", + "http 0.2.12", + "http 1.4.0", + "http-body 0.4.6", + "hyper 0.14.32", + "hyper 1.9.0", + "hyper-rustls 0.24.2", + "hyper-rustls 0.27.9", + "hyper-util", + "pin-project-lite", + "rustls 0.21.12", + "rustls 0.23.38", + "rustls-native-certs", + "rustls-pki-types", + "tokio", + "tokio-rustls 0.26.4", + "tower 0.5.3", + "tracing", +] + +[[package]] +name = "aws-smithy-json" +version = "0.62.7" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "701a947f4797e52a911e114a898667c746c39feea467bbd1abd7b3721f702ffa" +dependencies = [ + "aws-smithy-runtime-api", + "aws-smithy-schema", + "aws-smithy-types", +] + +[[package]] +name = "aws-smithy-observability" +version = "0.2.6" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "a06c2315d173edbf1920da8ba3a7189695827002e4c0fc961973ab1c54abca9c" +dependencies = [ + "aws-smithy-runtime-api", +] + +[[package]] +name = "aws-smithy-query" +version = "0.60.15" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "1a56d79744fb3edb5d722ef79d86081e121d3b9422cb209eb03aea6aa4f21ebd" +dependencies = [ + "aws-smithy-types", + "urlencoding", +] + +[[package]] +name = "aws-smithy-runtime" +version = "1.11.3" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "b8e6f5caf6fea86f8c2206541ab5857cfcda9013426cdbe8fa0098b9e2d32182" +dependencies = [ + "aws-smithy-async", + "aws-smithy-http", + "aws-smithy-http-client", + "aws-smithy-observability", + "aws-smithy-runtime-api", + "aws-smithy-schema", + "aws-smithy-types", + "bytes", + "fastrand", + "http 0.2.12", + "http 1.4.0", + "http-body 0.4.6", + "http-body 1.0.1", + "http-body-util", + "pin-project-lite", + "pin-utils", + "tokio", + "tracing", +] + [[package]] name = "aws-smithy-runtime-api" version = "1.12.3" @@ -385,6 +546,17 @@ dependencies = [ "syn 2.0.117", ] +[[package]] +name = "aws-smithy-schema" +version = "0.1.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "7442cb268338f0eb8278140a107c046756aa01093d8ef5e99628d34ae09c94f5" +dependencies = [ + "aws-smithy-runtime-api", + "aws-smithy-types", + "http 1.4.0", +] + [[package]] name = "aws-smithy-types" version = "1.5.0" @@ -394,6 +566,7 @@ dependencies = [ "base64-simd", "bytes", "bytes-utils", + "futures-core", "http 0.2.12", "http 1.4.0", "http-body 0.4.6", @@ -406,6 +579,32 @@ dependencies = [ "ryu", "serde", "time", + "tokio", + "tokio-util", +] + +[[package]] +name = "aws-smithy-xml" +version = "0.60.15" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "0ce02add1aa3677d022f8adf81dcbe3046a95f17a1b1e8979c145cd21d3d22b3" +dependencies = [ + "xmlparser", +] + +[[package]] +name = "aws-types" +version = "1.3.16" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "d16bf10b03a3c01e6b3b7d47cd964e873ffe9e7d4e80fad16bd4c077cb068531" +dependencies = [ + "aws-credential-types", + "aws-smithy-async", + "aws-smithy-runtime-api", + "aws-smithy-schema", + "aws-smithy-types", + "rustc_version", + "tracing", ] [[package]] @@ -422,7 +621,7 @@ dependencies = [ "http 1.4.0", "http-body 1.0.1", "http-body-util", - "hyper", + "hyper 1.9.0", "hyper-util", "itoa", "matchit", @@ -642,7 +841,7 @@ dependencies = [ "hex", "http 1.4.0", "http-body-util", - "hyper", + "hyper 1.9.0", "hyper-named-pipe", "hyper-util", "hyperlocal", @@ -1947,6 +2146,25 @@ dependencies = [ "subtle", ] +[[package]] +name = "h2" +version = "0.3.27" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "0beca50380b1fc32983fc1cb4587bfa4bb9e78fc259aad4a0032d2080309222d" +dependencies = [ + "bytes", + "fnv", + "futures-core", + "futures-sink", + "futures-util", + "http 0.2.12", + "indexmap", + "slab", + "tokio", + "tokio-util", + "tracing", +] + [[package]] name = "h2" version = "0.4.13" @@ -2184,6 +2402,30 @@ dependencies = [ "typenum", ] +[[package]] +name = "hyper" +version = "0.14.32" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "41dfc780fdec9373c01bae43289ea34c972e40ee3c9f6b3c8801a35f35586ce7" +dependencies = [ + "bytes", + "futures-channel", + "futures-core", + "futures-util", + "h2 0.3.27", + "http 0.2.12", + "http-body 0.4.6", + "httparse", + "httpdate", + "itoa", + "pin-project-lite", + "socket2 0.5.10", + "tokio", + "tower-service", + "tracing", + "want", +] + [[package]] name = "hyper" version = "1.9.0" @@ -2194,7 +2436,7 @@ dependencies = [ "bytes", "futures-channel", "futures-core", - "h2", + "h2 0.4.13", "http 1.4.0", "http-body 1.0.1", "httparse", @@ -2213,7 +2455,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "73b7d8abf35697b81a825e386fc151e0d503e8cb5fcb93cc8669c376dfd6f278" dependencies = [ "hex", - "hyper", + "hyper 1.9.0", "hyper-util", "pin-project-lite", "tokio", @@ -2221,6 +2463,21 @@ dependencies = [ "winapi", ] +[[package]] +name = "hyper-rustls" +version = "0.24.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "ec3efd23720e2049821a693cbc7e65ea87c72f1c58ff2f9522ff332b1491e590" +dependencies = [ + "futures-util", + "http 0.2.12", + "hyper 0.14.32", + "log", + "rustls 0.21.12", + "tokio", + "tokio-rustls 0.24.1", +] + [[package]] name = "hyper-rustls" version = "0.27.9" @@ -2228,13 +2485,13 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "33ca68d021ef39cf6463ab54c1d0f5daf03377b70561305bb89a8f83aab66e0f" dependencies = [ "http 1.4.0", - "hyper", + "hyper 1.9.0", "hyper-util", "log", - "rustls", + "rustls 0.23.38", "rustls-native-certs", "tokio", - "tokio-rustls", + "tokio-rustls 0.26.4", "tower-service", "webpki-roots 1.0.7", ] @@ -2245,7 +2502,7 @@ version = "0.5.2" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "2b90d566bffbce6a75bd8b09a05aa8c2cb1fabb6cb348f8840c9e4c90a0d83b0" dependencies = [ - "hyper", + "hyper 1.9.0", "hyper-util", "pin-project-lite", "tokio", @@ -2264,12 +2521,12 @@ dependencies = [ "futures-util", "http 1.4.0", "http-body 1.0.1", - "hyper", + "hyper 1.9.0", "ipnet", "libc", "percent-encoding", "pin-project-lite", - "socket2", + "socket2 0.6.3", "tokio", "tower-service", "tracing", @@ -2283,7 +2540,7 @@ checksum = "986c5ce3b994526b3cd75578e62554abd09f0899d6206de48b3e96ab34ccc8c7" dependencies = [ "hex", "http-body-util", - "hyper", + "hyper 1.9.0", "hyper-util", "pin-project-lite", "tokio", @@ -2810,15 +3067,15 @@ dependencies = [ "http 1.4.0", "http-body 1.0.1", "http-body-util", - "hyper", - "hyper-rustls", + "hyper 1.9.0", + "hyper-rustls 0.27.9", "hyper-timeout", "hyper-util", "jsonpath-rust", "k8s-openapi", "kube-core", "pem", - "rustls", + "rustls 0.23.38", "rustls-pemfile", "secrecy", "serde", @@ -3153,7 +3410,7 @@ checksum = "3589659543c04c7dc5526ec858591015b87cd8746583b51b48ef4353f99dbcda" dependencies = [ "base64 0.22.1", "http-body-util", - "hyper", + "hyper 1.9.0", "hyper-util", "indexmap", "ipnet", @@ -3587,8 +3844,8 @@ dependencies = [ "dialoguer", "futures", "http-body-util", - "hyper", - "hyper-rustls", + "hyper 1.9.0", + "hyper-rustls 0.27.9", "hyper-util", "indicatif", "miette", @@ -3604,7 +3861,7 @@ dependencies = [ "prost-types", "rcgen", "reqwest 0.12.28", - "rustls", + "rustls 0.23.38", "rustls-pemfile", "serde", "serde_json", @@ -3614,7 +3871,7 @@ dependencies = [ "tempfile", "thiserror 2.0.18", "tokio", - "tokio-rustls", + "tokio-rustls 0.26.4", "tokio-stream", "tokio-tungstenite 0.26.2", "tonic", @@ -3701,7 +3958,7 @@ dependencies = [ "clap", "futures", "http-body-util", - "hyper", + "hyper 1.9.0", "hyper-util", "miette", "nix", @@ -3834,7 +4091,7 @@ dependencies = [ "openshell-supervisor-network", "openshell-supervisor-process", "prost", - "rustls", + "rustls 0.23.38", "serde", "serde_json", "temp-env", @@ -3854,6 +4111,8 @@ dependencies = [ "anyhow", "arc-swap", "async-trait", + "aws-config", + "aws-sdk-sts", "axum", "bytes", "clap", @@ -3865,8 +4124,8 @@ dependencies = [ "http 1.4.0", "http-body 1.0.1", "http-body-util", - "hyper", - "hyper-rustls", + "hyper 1.9.0", + "hyper-rustls 0.27.9", "hyper-util", "ipnet", "jsonwebtoken 9.3.1", @@ -3896,7 +4155,7 @@ dependencies = [ "reqwest 0.12.28", "russh", "rustix 1.1.4", - "rustls", + "rustls 0.23.38", "rustls-pemfile", "serde", "serde_json", @@ -3905,7 +4164,7 @@ dependencies = [ "tempfile", "thiserror 2.0.18", "tokio", - "tokio-rustls", + "tokio-rustls 0.26.4", "tokio-stream", "tokio-tungstenite 0.26.2", "toml", @@ -3954,7 +4213,7 @@ dependencies = [ "rcgen", "regorus", "reqwest 0.12.28", - "rustls", + "rustls 0.23.38", "rustls-pemfile", "serde", "serde_json", @@ -3966,7 +4225,7 @@ dependencies = [ "tempfile", "thiserror 2.0.18", "tokio", - "tokio-rustls", + "tokio-rustls 0.26.4", "tokio-tungstenite 0.26.2", "tower-mcp-types", "tracing", @@ -4631,8 +4890,8 @@ dependencies = [ "quinn-proto", "quinn-udp", "rustc-hash 2.1.2", - "rustls", - "socket2", + "rustls 0.23.38", + "socket2 0.6.3", "thiserror 2.0.18", "tokio", "tracing", @@ -4652,7 +4911,7 @@ dependencies = [ "rand 0.9.4", "ring", "rustc-hash 2.1.2", - "rustls", + "rustls 0.23.38", "rustls-pki-types", "slab", "thiserror 2.0.18", @@ -4670,7 +4929,7 @@ dependencies = [ "cfg_aliases", "libc", "once_cell", - "socket2", + "socket2 0.6.3", "tracing", "windows-sys 0.60.2", ] @@ -4853,6 +5112,12 @@ dependencies = [ "regex-syntax", ] +[[package]] +name = "regex-lite" +version = "0.1.9" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "cab834c73d247e67f4fae452806d17d3c7501756d98c8808d7c9c7aa7d18f973" + [[package]] name = "regex-syntax" version = "0.8.10" @@ -4892,15 +5157,15 @@ dependencies = [ "http 1.4.0", "http-body 1.0.1", "http-body-util", - "hyper", - "hyper-rustls", + "hyper 1.9.0", + "hyper-rustls 0.27.9", "hyper-util", "js-sys", "log", "percent-encoding", "pin-project-lite", "quinn", - "rustls", + "rustls 0.23.38", "rustls-native-certs", "rustls-pki-types", "serde", @@ -4908,7 +5173,7 @@ dependencies = [ "serde_urlencoded", "sync_wrapper", "tokio", - "tokio-rustls", + "tokio-rustls 0.26.4", "tower 0.5.3", "tower-http 0.6.8", "tower-service", @@ -4932,15 +5197,15 @@ dependencies = [ "http 1.4.0", "http-body 1.0.1", "http-body-util", - "hyper", - "hyper-rustls", + "hyper 1.9.0", + "hyper-rustls 0.27.9", "hyper-util", "js-sys", "log", "percent-encoding", "pin-project-lite", "quinn", - "rustls", + "rustls 0.23.38", "rustls-pki-types", "rustls-platform-verifier", "serde", @@ -4948,7 +5213,7 @@ dependencies = [ "serde_urlencoded", "sync_wrapper", "tokio", - "tokio-rustls", + "tokio-rustls 0.26.4", "tokio-util", "tower 0.5.3", "tower-http 0.6.8", @@ -5184,6 +5449,18 @@ dependencies = [ "windows-sys 0.61.2", ] +[[package]] +name = "rustls" +version = "0.21.12" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "3f56a14d1f48b391359b22f731fd4bd7e43c97f3c50eee276f3aa09c94784d3e" +dependencies = [ + "log", + "ring", + "rustls-webpki 0.101.7", + "sct", +] + [[package]] name = "rustls" version = "0.23.38" @@ -5195,7 +5472,7 @@ dependencies = [ "once_cell", "ring", "rustls-pki-types", - "rustls-webpki", + "rustls-webpki 0.103.13", "subtle", "zeroize", ] @@ -5242,10 +5519,10 @@ dependencies = [ "jni", "log", "once_cell", - "rustls", + "rustls 0.23.38", "rustls-native-certs", "rustls-platform-verifier-android", - "rustls-webpki", + "rustls-webpki 0.103.13", "security-framework", "security-framework-sys", "webpki-root-certs", @@ -5258,6 +5535,16 @@ version = "0.1.1" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "f87165f0995f63a9fbeea62b64d10b4d9d8e78ec6d7d51fb2125fda7bb36788f" +[[package]] +name = "rustls-webpki" +version = "0.101.7" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "8b6275d1ee7a1cd780b64aca7726599a1dbc893b1e64144529e55c3c2f745765" +dependencies = [ + "ring", + "untrusted 0.9.0", +] + [[package]] name = "rustls-webpki" version = "0.103.13" @@ -5350,6 +5637,16 @@ dependencies = [ "sha2 0.10.9", ] +[[package]] +name = "sct" +version = "0.7.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "da046153aa2352493d6cb7da4b6e5c0c057d8a1d0a9aa8560baffdd945acd414" +dependencies = [ + "ring", + "untrusted 0.9.0", +] + [[package]] name = "sec1" version = "0.7.3" @@ -5719,6 +6016,16 @@ dependencies = [ "serde", ] +[[package]] +name = "socket2" +version = "0.5.10" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "e22376abed350d73dd1cd119b57ffccad95b4e585a7cda43e286245ce23c0678" +dependencies = [ + "libc", + "windows-sys 0.52.0", +] + [[package]] name = "socket2" version = "0.6.3" @@ -5822,7 +6129,7 @@ dependencies = [ "memchr", "once_cell", "percent-encoding", - "rustls", + "rustls 0.23.38", "serde", "serde_json", "sha2 0.10.9", @@ -6383,7 +6690,7 @@ dependencies = [ "parking_lot", "pin-project-lite", "signal-hook-registry", - "socket2", + "socket2 0.6.3", "tokio-macros", "windows-sys 0.61.2", ] @@ -6399,13 +6706,23 @@ dependencies = [ "syn 2.0.117", ] +[[package]] +name = "tokio-rustls" +version = "0.24.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "c28327cf380ac148141087fbfb9de9d7bd4e84ab5d2c28fbc911d753de8a7081" +dependencies = [ + "rustls 0.21.12", + "tokio", +] + [[package]] name = "tokio-rustls" version = "0.26.4" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "1729aa945f29d91ba541258c8df89027d5792d85a8841fb65e8bf0f4ede4ef61" dependencies = [ - "rustls", + "rustls 0.23.38", "tokio", ] @@ -6428,11 +6745,11 @@ checksum = "7a9daff607c6d2bf6c16fd681ccb7eecc83e4e2cdc1ca067ffaadfca5de7f084" dependencies = [ "futures-util", "log", - "rustls", + "rustls 0.23.38", "rustls-native-certs", "rustls-pki-types", "tokio", - "tokio-rustls", + "tokio-rustls 0.26.4", "tungstenite 0.26.2", ] @@ -6513,20 +6830,20 @@ dependencies = [ "axum", "base64 0.22.1", "bytes", - "h2", + "h2 0.4.13", "http 1.4.0", "http-body 1.0.1", "http-body-util", - "hyper", + "hyper 1.9.0", "hyper-timeout", "hyper-util", "percent-encoding", "pin-project", "rustls-native-certs", - "socket2", + "socket2 0.6.3", "sync_wrapper", "tokio", - "tokio-rustls", + "tokio-rustls 0.26.4", "tokio-stream", "tower 0.5.3", "tower-layer", @@ -6778,7 +7095,7 @@ dependencies = [ "httparse", "log", "rand 0.9.4", - "rustls", + "rustls 0.23.38", "rustls-pki-types", "sha1 0.10.6", "thiserror 2.0.18", @@ -6934,6 +7251,12 @@ dependencies = [ "serde_derive", ] +[[package]] +name = "urlencoding" +version = "2.1.3" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "daf8dba3b7eb870caf1ddeed7bc9d2a049f3cfdfae7cb521b087cc33ae4c49da" + [[package]] name = "utf-8" version = "0.7.6" @@ -7654,7 +7977,7 @@ dependencies = [ "futures", "http 1.4.0", "http-body-util", - "hyper", + "hyper 1.9.0", "hyper-util", "log", "once_cell", @@ -7792,6 +8115,12 @@ dependencies = [ "rustix 1.1.4", ] +[[package]] +name = "xmlparser" +version = "0.13.6" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "66fee0b777b0f5ac1c69bb06d361268faafa61cd4682ae064a171c16c433e9e4" + [[package]] name = "xterm-color" version = "1.0.2" diff --git a/Cargo.toml b/Cargo.toml index f450cd5c8c..4d1e52825b 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -78,6 +78,10 @@ tower-mcp-types = "0.12.0" # HTTP client reqwest = { version = "0.12", default-features = false, features = ["json", "rustls-tls-native-roots"] } +# AWS SDK +aws-config = { version = "1", default-features = false, features = ["rustls", "rt-tokio", "behavior-version-latest"] } +aws-sdk-sts = { version = "1", default-features = false, features = ["rustls", "rt-tokio", "behavior-version-latest"] } + # WebSocket tokio-tungstenite = { version = "0.26", features = ["rustls-tls-native-roots"] } diff --git a/architecture/security-policy.md b/architecture/security-policy.md index c4eaa2328a..10a8400fb9 100644 --- a/architecture/security-policy.md +++ b/architecture/security-policy.md @@ -46,18 +46,21 @@ request is denied. ## Host Wildcards Network endpoint `host` patterns accept a `*` wildcard inside the first DNS -label only. The OPA runtime matches with a `.` label boundary, so a wildcard -never spans dots. The validator enforces the same boundary so that policy load -fails fast instead of silently mismatching at the proxy. +label and as an entire middle DNS label. The OPA runtime matches with a `.` +label boundary, so a wildcard never spans dots. The validator enforces the same +boundary so that policy load fails fast instead of silently mismatching at the +proxy. | Pattern | Accepted | Example match | Notes | |---|---|---|---| | `*.example.com` | Yes | `api.example.com` | Single first label of any value. | | `**.example.com` | Yes | `a.b.example.com` | Recursive wildcard as the entire first label. | | `*-aiplatform.googleapis.com` | Yes | `us-central1-aiplatform.googleapis.com` | Intra-label wildcard inside the first DNS label. | +| `*.s3.*.amazonaws.com` | Yes | `bucket.s3.us-east-1.amazonaws.com` | Middle-label `*` matches exactly one DNS label. | | `*` or `**` | No | — | Matches every host. | | `*.com`, `**.com` | No | — | TLD wildcards (`labels <= 2`). | -| `foo.*.example.com` | No | — | Wildcard outside the first DNS label. | +| `foo.us-*.example.com` | No | — | Partial middle-label wildcards are not allowed. | +| `foo.**.example.com` | No | — | Recursive wildcard outside the first label is not allowed. | | `foo**.example.com` | No | — | Recursive `**` mixed inside a label; allowed only as the entire first label. | Validation rejects the disallowed patterns at policy load time with a message diff --git a/crates/openshell-cli/src/main.rs b/crates/openshell-cli/src/main.rs index 66484562d4..d135438676 100644 --- a/crates/openshell-cli/src/main.rs +++ b/crates/openshell-cli/src/main.rs @@ -692,6 +692,7 @@ enum CliProviderRefreshStrategy { Oauth2RefreshToken, Oauth2ClientCredentials, GoogleServiceAccountJwt, + AwsStsAssumeRole, } impl CliProviderRefreshStrategy { @@ -700,6 +701,7 @@ impl CliProviderRefreshStrategy { Self::Oauth2RefreshToken => "oauth2_refresh_token", Self::Oauth2ClientCredentials => "oauth2_client_credentials", Self::GoogleServiceAccountJwt => "google_service_account_jwt", + Self::AwsStsAssumeRole => "aws_sts_assume_role", } } } diff --git a/crates/openshell-cli/src/run.rs b/crates/openshell-cli/src/run.rs index 0182e1b668..2e73f686a2 100644 --- a/crates/openshell-cli/src/run.rs +++ b/crates/openshell-cli/src/run.rs @@ -5447,6 +5447,7 @@ fn provider_refresh_strategy(strategy: &str) -> Result { Ok(ProviderCredentialRefreshStrategy::GoogleServiceAccountJwt) } + "aws_sts_assume_role" => Ok(ProviderCredentialRefreshStrategy::AwsStsAssumeRole), _ => Err(miette!("unsupported provider refresh strategy: {strategy}")), } } @@ -5499,6 +5500,7 @@ fn provider_refresh_strategy_name(strategy: ProviderCredentialRefreshStrategy) - ProviderCredentialRefreshStrategy::Oauth2RefreshToken => "oauth2_refresh_token", ProviderCredentialRefreshStrategy::Oauth2ClientCredentials => "oauth2_client_credentials", ProviderCredentialRefreshStrategy::GoogleServiceAccountJwt => "google_service_account_jwt", + ProviderCredentialRefreshStrategy::AwsStsAssumeRole => "aws_sts_assume_role", ProviderCredentialRefreshStrategy::Unspecified => "unspecified", } } diff --git a/crates/openshell-policy/src/lib.rs b/crates/openshell-policy/src/lib.rs index f1721146ed..8caf7de3ac 100644 --- a/crates/openshell-policy/src/lib.rs +++ b/crates/openshell-policy/src/lib.rs @@ -1109,6 +1109,8 @@ pub enum PolicyViolation { TooManyPaths { count: usize }, /// A network endpoint uses a TLD wildcard (e.g. `*.com`). TldWildcard { policy_name: String, host: String }, + /// A network endpoint uses a wildcard shape that does not match runtime semantics. + InvalidHostWildcard { policy_name: String, host: String }, /// `credential_signing` is set but `signing_service` is missing. MissingSigningService { policy_name: String, host: String }, /// `credential_signing` has an unrecognized value. @@ -1158,6 +1160,14 @@ impl fmt::Display for PolicyViolation { use subdomain wildcards like '*.example.com' instead" ) } + Self::InvalidHostWildcard { policy_name, host } => { + write!( + f, + "network policy '{policy_name}': invalid host wildcard '{host}'; \ + middle DNS label wildcards must be the entire label '*' and recursive '**' \ + is only allowed as the entire first label" + ) + } Self::MissingSigningService { policy_name, host } => { write!( f, @@ -1288,6 +1298,12 @@ pub fn validate_sandbox_policy( }); } } + if host_wildcard_shape_invalid(&ep.host) { + violations.push(PolicyViolation::InvalidHostWildcard { + policy_name: name.clone(), + host: ep.host.clone(), + }); + } if !ep.credential_signing.is_empty() && !matches!( ep.credential_signing.as_str(), @@ -1322,6 +1338,25 @@ pub fn validate_sandbox_policy( } } +fn host_wildcard_shape_invalid(host: &str) -> bool { + if host == "*" || host == "**" { + return true; + } + if !host.contains('*') { + return false; + } + let labels: Vec<&str> = host.split('.').collect(); + let first_label = labels.first().copied().unwrap_or_default(); + if first_label.contains("**") && first_label != "**" { + return true; + } + labels + .iter() + .skip(1) + .copied() + .any(|label| label.contains("**") || (label.contains('*') && label != "*")) +} + /// Truncate a string for safe inclusion in error messages. fn truncate_for_display(s: &str) -> String { if s.len() <= 80 { @@ -1848,6 +1883,33 @@ network_policies: ); } + #[test] + fn validate_rejects_all_host_star_wildcards() { + for host in ["*", "**"] { + let mut policy = restrictive_default_policy(); + policy.network_policies.insert( + "bad".into(), + NetworkPolicyRule { + name: "bad-rule".into(), + endpoints: vec![NetworkEndpoint { + host: host.into(), + port: 443, + ..Default::default() + }], + ..Default::default() + }, + ); + + let violations = validate_sandbox_policy(&policy).unwrap_err(); + assert!( + violations + .iter() + .any(|v| matches!(v, PolicyViolation::InvalidHostWildcard { .. })), + "expected bare host wildcard {host:?} to be rejected, got {violations:?}" + ); + } + } + #[test] fn validate_accepts_subdomain_wildcard() { let mut policy = restrictive_default_policy(); @@ -1866,6 +1928,47 @@ network_policies: assert!(validate_sandbox_policy(&policy).is_ok()); } + #[test] + fn validate_accepts_middle_label_star_wildcard() { + let mut policy = restrictive_default_policy(); + policy.network_policies.insert( + "ok".into(), + NetworkPolicyRule { + name: "ok-rule".into(), + endpoints: vec![NetworkEndpoint { + host: "*.s3.*.amazonaws.com".into(), + port: 443, + ..Default::default() + }], + ..Default::default() + }, + ); + assert!(validate_sandbox_policy(&policy).is_ok()); + } + + #[test] + fn validate_rejects_partial_middle_label_wildcard() { + let mut policy = restrictive_default_policy(); + policy.network_policies.insert( + "bad".into(), + NetworkPolicyRule { + name: "bad-rule".into(), + endpoints: vec![NetworkEndpoint { + host: "*.s3.us-*.amazonaws.com".into(), + port: 443, + ..Default::default() + }], + ..Default::default() + }, + ); + let violations = validate_sandbox_policy(&policy).unwrap_err(); + assert!( + violations + .iter() + .any(|v| matches!(v, PolicyViolation::InvalidHostWildcard { .. })) + ); + } + #[test] fn validate_accepts_explicit_domain() { let mut policy = restrictive_default_policy(); diff --git a/crates/openshell-providers/src/profiles.rs b/crates/openshell-providers/src/profiles.rs index ddfbcaf7dc..4e03cdef9d 100644 --- a/crates/openshell-providers/src/profiles.rs +++ b/crates/openshell-providers/src/profiles.rs @@ -21,7 +21,9 @@ use std::sync::OnceLock; const PATH_TEMPLATE_CREDENTIAL_PLACEHOLDER: &str = "{credential}"; const BUILT_IN_PROFILE_YAMLS: &[&str] = &[ + include_str!("../../../providers/aws.yaml"), include_str!("../../../providers/aws-bedrock.yaml"), + include_str!("../../../providers/aws-s3.yaml"), include_str!("../../../providers/claude-code.yaml"), include_str!("../../../providers/codex.yaml"), include_str!("../../../providers/copilot.yaml"), @@ -639,6 +641,7 @@ pub fn provider_refresh_strategy_from_yaml(raw: &str) -> Option { Some(ProviderCredentialRefreshStrategy::GoogleServiceAccountJwt) } + "aws_sts_assume_role" => Some(ProviderCredentialRefreshStrategy::AwsStsAssumeRole), _ => None, } } @@ -653,6 +656,7 @@ pub fn provider_refresh_strategy_to_yaml( ProviderCredentialRefreshStrategy::Oauth2RefreshToken => "oauth2_refresh_token", ProviderCredentialRefreshStrategy::Oauth2ClientCredentials => "oauth2_client_credentials", ProviderCredentialRefreshStrategy::GoogleServiceAccountJwt => "google_service_account_jwt", + ProviderCredentialRefreshStrategy::AwsStsAssumeRole => "aws_sts_assume_role", ProviderCredentialRefreshStrategy::Unspecified => "unspecified", } } @@ -2690,4 +2694,92 @@ endpoints: assert!(matches!(err, ProfileError::InvalidEndpoint { id, .. } if id == "bad-endpoint")); } + + #[test] + fn aws_sts_strategy_serde_roundtrip() { + use openshell_core::proto::ProviderCredentialRefreshStrategy; + assert_eq!( + super::provider_refresh_strategy_from_yaml("aws_sts_assume_role"), + Some(ProviderCredentialRefreshStrategy::AwsStsAssumeRole) + ); + assert_eq!( + super::provider_refresh_strategy_to_yaml( + ProviderCredentialRefreshStrategy::AwsStsAssumeRole + ), + "aws_sts_assume_role" + ); + } + + #[test] + fn aws_profile_parses_correctly() { + let aws = get_default_profile("aws").expect("aws profile should exist"); + assert_eq!(aws.display_name, "AWS"); + assert_eq!(aws.credentials.len(), 3); + let access_key = aws + .credentials + .iter() + .find(|c| c.name == "access_key_id") + .unwrap(); + assert!(access_key.refresh.is_some()); + let refresh = access_key.refresh.as_ref().unwrap(); + assert_eq!( + refresh.strategy, + openshell_core::proto::ProviderCredentialRefreshStrategy::AwsStsAssumeRole + ); + assert!( + refresh + .material + .iter() + .any(|m| m.name == "role_arn" && m.required) + ); + } + + #[test] + fn aws_s3_profile_parses_with_endpoints() { + let aws_s3 = get_default_profile("aws-s3").expect("aws-s3 profile should exist"); + assert_eq!(aws_s3.display_name, "AWS S3"); + assert!(!aws_s3.endpoints.is_empty()); + assert!( + !aws_s3 + .endpoints + .iter() + .any(|e| e.host == "**.amazonaws.com") + ); + assert!( + aws_s3 + .endpoints + .iter() + .any(|e| e.host == "*.s3.amazonaws.com") + ); + assert!( + aws_s3 + .endpoints + .iter() + .any(|e| e.host == "s3.amazonaws.com") + ); + assert!( + aws_s3 + .endpoints + .iter() + .any(|e| e.host == "*.s3.*.amazonaws.com") + ); + assert!( + aws_s3 + .endpoints + .iter() + .any(|e| e.host == "s3.*.amazonaws.com") + ); + assert!( + aws_s3 + .endpoints + .iter() + .any(|e| e.host == "*.s3.dualstack.*.amazonaws.com") + ); + assert!( + aws_s3 + .endpoints + .iter() + .any(|e| e.host == "s3.dualstack.*.amazonaws.com") + ); + } } diff --git a/crates/openshell-server/Cargo.toml b/crates/openshell-server/Cargo.toml index b5c9b34d71..1d0640e371 100644 --- a/crates/openshell-server/Cargo.toml +++ b/crates/openshell-server/Cargo.toml @@ -80,6 +80,8 @@ toml = { workspace = true } tokio-stream = { workspace = true } sqlx = { workspace = true } reqwest = { workspace = true } +aws-config = { workspace = true } +aws-sdk-sts = { workspace = true } uuid = { workspace = true } hmac = "0.12" sha2 = { workspace = true } diff --git a/crates/openshell-server/src/grpc/policy.rs b/crates/openshell-server/src/grpc/policy.rs index c016531eb6..563f720c77 100644 --- a/crates/openshell-server/src/grpc/policy.rs +++ b/crates/openshell-server/src/grpc/policy.rs @@ -1395,7 +1395,7 @@ async fn profile_provider_policy_layers( Ok(layers) } -fn bool_setting_enabled(settings: &StoredSettings, key: &str) -> Result { +pub(super) fn bool_setting_enabled(settings: &StoredSettings, key: &str) -> Result { match settings.settings.get(key) { None => Ok(false), Some(StoredSettingValue::Bool(value)) => Ok(*value), diff --git a/crates/openshell-server/src/grpc/provider.rs b/crates/openshell-server/src/grpc/provider.rs index d5a5f5c909..04a15dcbb4 100644 --- a/crates/openshell-server/src/grpc/provider.rs +++ b/crates/openshell-server/src/grpc/provider.rs @@ -1996,6 +1996,18 @@ pub(super) async fn handle_configure_provider_refresh( crate::provider_refresh::refresh_strategy_name(strategy as i32) ))); } + if strategy == ProviderCredentialRefreshStrategy::AwsStsAssumeRole { + let global_settings = + crate::grpc::policy::load_global_settings(state.store.as_ref()).await?; + if !crate::grpc::policy::bool_setting_enabled( + &global_settings, + openshell_core::settings::PROVIDERS_V2_ENABLED_KEY, + )? { + return Err(Status::failed_precondition( + "aws_sts_assume_role requires providers_v2_enabled=true", + )); + } + } if request.material.len() > MAX_PROVIDER_CONFIG_ENTRIES { return Err(Status::invalid_argument(format!( "material exceeds maximum entries ({} > {MAX_PROVIDER_CONFIG_ENTRIES})", @@ -2064,6 +2076,16 @@ pub(super) async fn handle_configure_provider_refresh( credential_key, ) .await?; + if strategy == ProviderCredentialRefreshStrategy::AwsStsAssumeRole { + for additional_key in ["AWS_SECRET_ACCESS_KEY", "AWS_SESSION_TOKEN"] { + validate_provider_credential_key_available_for_attached_sandboxes( + state.store.as_ref(), + &provider, + additional_key, + ) + .await?; + } + } let refresh_defaults = provider_refresh_defaults(state.store.as_ref(), &provider, credential_key).await?; validate_refresh_material(&request.material, refresh_defaults.as_ref())?; @@ -3142,7 +3164,9 @@ mod tests { assert_eq!( ids, vec![ + "aws", "aws-bedrock", + "aws-s3", "claude-code", "codex", "copilot", @@ -6029,6 +6053,230 @@ mod tests { assert_eq!(new_keys_count, 1); } + #[tokio::test] + async fn configure_aws_sts_requires_v2_enabled() { + let state = test_server_state().await; + create_provider_record( + state.store.as_ref(), + Provider { + metadata: Some(openshell_core::proto::datamodel::v1::ObjectMeta { + id: String::new(), + name: "my-aws".to_string(), + created_at_ms: 0, + labels: HashMap::new(), + resource_version: 0, + }), + r#type: "aws".to_string(), + credentials: std::iter::once(( + "AWS_ACCESS_KEY_ID".to_string(), + "placeholder".to_string(), + )) + .collect(), + config: HashMap::new(), + credential_expires_at_ms: HashMap::new(), + }, + ) + .await + .unwrap(); + + let err = handle_configure_provider_refresh( + &state, + Request::new(ConfigureProviderRefreshRequest { + provider: "my-aws".to_string(), + credential_key: "AWS_ACCESS_KEY_ID".to_string(), + strategy: ProviderCredentialRefreshStrategy::AwsStsAssumeRole as i32, + material: HashMap::from([( + "role_arn".to_string(), + "arn:aws:iam::123456789012:role/Test".to_string(), + )]), + secret_material_keys: Vec::new(), + expires_at_ms: None, + }), + ) + .await + .unwrap_err(); + + assert_eq!(err.code(), Code::FailedPrecondition); + assert!(err.message().contains("providers_v2_enabled")); + } + + #[tokio::test] + async fn configure_aws_sts_succeeds_with_v2_enabled() { + use crate::grpc::StoredSettingValue; + use crate::grpc::StoredSettings; + use crate::grpc::policy::save_global_settings; + + let state = test_server_state().await; + + let global_settings = StoredSettings { + revision: 1, + settings: std::iter::once(( + openshell_core::settings::PROVIDERS_V2_ENABLED_KEY.to_string(), + StoredSettingValue::Bool(true), + )) + .collect(), + ..Default::default() + }; + save_global_settings(state.store.as_ref(), &global_settings) + .await + .unwrap(); + + create_provider_record( + state.store.as_ref(), + Provider { + metadata: Some(openshell_core::proto::datamodel::v1::ObjectMeta { + id: String::new(), + name: "my-aws-v2".to_string(), + created_at_ms: 0, + labels: HashMap::new(), + resource_version: 0, + }), + r#type: "aws".to_string(), + credentials: std::iter::once(( + "AWS_ACCESS_KEY_ID".to_string(), + "placeholder".to_string(), + )) + .collect(), + config: HashMap::new(), + credential_expires_at_ms: HashMap::new(), + }, + ) + .await + .unwrap(); + + let response = handle_configure_provider_refresh( + &state, + Request::new(ConfigureProviderRefreshRequest { + provider: "my-aws-v2".to_string(), + credential_key: "AWS_ACCESS_KEY_ID".to_string(), + strategy: ProviderCredentialRefreshStrategy::AwsStsAssumeRole as i32, + material: HashMap::from([( + "role_arn".to_string(), + "arn:aws:iam::123456789012:role/Test".to_string(), + )]), + secret_material_keys: Vec::new(), + expires_at_ms: None, + }), + ) + .await + .unwrap() + .into_inner() + .status + .expect("status"); + + assert_eq!(response.credential_key, "AWS_ACCESS_KEY_ID"); + assert_eq!( + response.strategy, + ProviderCredentialRefreshStrategy::AwsStsAssumeRole as i32 + ); + } + + #[tokio::test] + async fn configure_aws_sts_validates_additional_credential_key_collision() { + use crate::grpc::StoredSettingValue; + use crate::grpc::StoredSettings; + use crate::grpc::policy::save_global_settings; + + let state = test_server_state().await; + + let global_settings = StoredSettings { + revision: 1, + settings: std::iter::once(( + openshell_core::settings::PROVIDERS_V2_ENABLED_KEY.to_string(), + StoredSettingValue::Bool(true), + )) + .collect(), + ..Default::default() + }; + save_global_settings(state.store.as_ref(), &global_settings) + .await + .unwrap(); + + let mut existing_provider = Provider { + metadata: Some(openshell_core::proto::datamodel::v1::ObjectMeta { + id: String::new(), + name: "existing-aws-provider".to_string(), + created_at_ms: 0, + labels: HashMap::new(), + resource_version: 0, + }), + r#type: "aws".to_string(), + credentials: HashMap::new(), + config: HashMap::new(), + credential_expires_at_ms: HashMap::new(), + }; + existing_provider.credentials.insert( + "AWS_SECRET_ACCESS_KEY".to_string(), + "existing-key".to_string(), + ); + create_provider_record(state.store.as_ref(), existing_provider) + .await + .unwrap(); + + let new_provider = Provider { + metadata: Some(openshell_core::proto::datamodel::v1::ObjectMeta { + id: String::new(), + name: "new-aws-provider".to_string(), + created_at_ms: 0, + labels: HashMap::new(), + resource_version: 0, + }), + r#type: "aws".to_string(), + credentials: std::iter::once(( + "AWS_ACCESS_KEY_ID".to_string(), + "placeholder".to_string(), + )) + .collect(), + config: HashMap::new(), + credential_expires_at_ms: HashMap::new(), + }; + create_provider_record(state.store.as_ref(), new_provider) + .await + .unwrap(); + + state + .store + .put_message(&Sandbox { + metadata: Some(openshell_core::proto::datamodel::v1::ObjectMeta { + id: "sandbox-aws-configure-collision".to_string(), + name: "aws-configure-collision".to_string(), + created_at_ms: 1, + labels: HashMap::new(), + resource_version: 0, + }), + spec: Some(SandboxSpec { + providers: vec![ + "existing-aws-provider".to_string(), + "new-aws-provider".to_string(), + ], + ..SandboxSpec::default() + }), + ..Default::default() + }) + .await + .unwrap(); + + let err = handle_configure_provider_refresh( + &state, + Request::new(ConfigureProviderRefreshRequest { + provider: "new-aws-provider".to_string(), + credential_key: "AWS_ACCESS_KEY_ID".to_string(), + strategy: ProviderCredentialRefreshStrategy::AwsStsAssumeRole as i32, + material: HashMap::from([( + "role_arn".to_string(), + "arn:aws:iam::123456789012:role/Test".to_string(), + )]), + secret_material_keys: Vec::new(), + expires_at_ms: None, + }), + ) + .await + .unwrap_err(); + + assert_eq!(err.code(), Code::FailedPrecondition); + assert!(err.message().contains("AWS_SECRET_ACCESS_KEY")); + } + fn google_cloud_provider(config: HashMap) -> Provider { Provider { metadata: Some(openshell_core::proto::datamodel::v1::ObjectMeta { diff --git a/crates/openshell-server/src/provider_refresh.rs b/crates/openshell-server/src/provider_refresh.rs index b0b9a927c4..9ed9f1f899 100644 --- a/crates/openshell-server/src/provider_refresh.rs +++ b/crates/openshell-server/src/provider_refresh.rs @@ -226,6 +226,7 @@ struct MintedCredential { access_token: String, expires_at_ms: i64, refresh_token: Option, + additional_credentials: HashMap, } #[derive(Debug, Deserialize)] @@ -279,6 +280,7 @@ pub fn refresh_strategy_name(strategy: i32) -> &'static str { ProviderCredentialRefreshStrategy::Oauth2RefreshToken => "oauth2_refresh_token", ProviderCredentialRefreshStrategy::Oauth2ClientCredentials => "oauth2_client_credentials", ProviderCredentialRefreshStrategy::GoogleServiceAccountJwt => "google_service_account_jwt", + ProviderCredentialRefreshStrategy::AwsStsAssumeRole => "aws_sts_assume_role", ProviderCredentialRefreshStrategy::Unspecified => "unspecified", } } @@ -289,6 +291,7 @@ pub fn is_gateway_mintable_strategy(strategy: ProviderCredentialRefreshStrategy) ProviderCredentialRefreshStrategy::Oauth2RefreshToken | ProviderCredentialRefreshStrategy::Oauth2ClientCredentials | ProviderCredentialRefreshStrategy::GoogleServiceAccountJwt + | ProviderCredentialRefreshStrategy::AwsStsAssumeRole ) } @@ -407,12 +410,23 @@ async fn apply_minted_credential( updated .credentials .insert(credential_key.to_string(), minted.access_token.clone()); + for (key, value) in &minted.additional_credentials { + updated.credentials.insert(key.clone(), value.clone()); + } if minted.expires_at_ms > 0 { updated .credential_expires_at_ms .insert(credential_key.to_string(), minted.expires_at_ms); + for key in minted.additional_credentials.keys() { + updated + .credential_expires_at_ms + .insert(key.clone(), minted.expires_at_ms); + } } else { updated.credential_expires_at_ms.remove(credential_key); + for key in minted.additional_credentials.keys() { + updated.credential_expires_at_ms.remove(key); + } } crate::grpc::provider::validate_provider_update_against_attached_sandboxes(store, &updated) .await?; @@ -421,12 +435,23 @@ async fn apply_minted_credential( current .credentials .insert(credential_key.to_string(), minted.access_token.clone()); + for (key, value) in &minted.additional_credentials { + current.credentials.insert(key.clone(), value.clone()); + } if minted.expires_at_ms > 0 { current .credential_expires_at_ms .insert(credential_key.to_string(), minted.expires_at_ms); + for key in minted.additional_credentials.keys() { + current + .credential_expires_at_ms + .insert(key.clone(), minted.expires_at_ms); + } } else { current.credential_expires_at_ms.remove(credential_key); + for key in minted.additional_credentials.keys() { + current.credential_expires_at_ms.remove(key); + } } }) .await @@ -449,6 +474,9 @@ async fn mint_credential( ProviderCredentialRefreshStrategy::GoogleServiceAccountJwt => { mint_google_service_account_jwt(state).await } + ProviderCredentialRefreshStrategy::AwsStsAssumeRole => { + mint_aws_sts_assume_role(state).await + } ProviderCredentialRefreshStrategy::External | ProviderCredentialRefreshStrategy::Static | ProviderCredentialRefreshStrategy::Unspecified => Err(Status::failed_precondition( @@ -544,6 +572,97 @@ async fn mint_google_service_account_jwt( request_token(&token_url, &form, lifetime_secs).await } +async fn mint_aws_sts_assume_role( + state: &StoredProviderCredentialRefreshState, +) -> Result { + let role_arn = required_material(&state.material, "role_arn")?; + let session_name = material_value(&state.material, &["session_name"]) + .unwrap_or_else(|| "openshell-sandbox".to_string()); + let external_id = material_value(&state.material, &["external_id"]); + let region = + material_value(&state.material, &["aws_region"]).unwrap_or_else(|| "us-east-1".to_string()); + + let region_provider = aws_sdk_sts::config::Region::new(region); + let mut config_loader = + aws_config::defaults(aws_config::BehaviorVersion::latest()).region(region_provider); + + if let (Some(access_key), Some(secret_key)) = ( + material_value(&state.material, &["aws_access_key_id"]), + material_value(&state.material, &["aws_secret_access_key"]), + ) { + let creds = aws_sdk_sts::config::Credentials::new( + access_key, + secret_key, + None, + None, + "openshell-provider-refresh", + ); + config_loader = config_loader.credentials_provider(creds); + } + + let sdk_config = config_loader.load().await; + let sts_config = { + let mut builder = aws_sdk_sts::config::Builder::from(&sdk_config); + if let Some(endpoint) = material_value(&state.material, &["sts_endpoint_url"]) + && let Ok(parsed) = reqwest::Url::parse(&endpoint) + && parsed.host_str().is_some_and(is_loopback_host) + { + builder = builder.endpoint_url(endpoint); + } + builder.build() + }; + let client = aws_sdk_sts::Client::from_conf(sts_config); + + let max_lifetime_i64 = if state.max_lifetime_seconds > 0 { + state.max_lifetime_seconds + } else { + DEFAULT_MAX_LIFETIME_SECONDS + }; + let max_lifetime = i32::try_from(max_lifetime_i64.min(i64::from(i32::MAX))).unwrap_or(i32::MAX); + + let mut req = client + .assume_role() + .role_arn(&role_arn) + .role_session_name(&session_name) + .duration_seconds(max_lifetime); + + if let Some(eid) = external_id { + req = req.external_id(eid); + } + + let resp = req + .send() + .await + .map_err(|e| Status::internal(format!("STS AssumeRole failed: {e}")))?; + + let creds = resp + .credentials() + .ok_or_else(|| Status::internal("STS AssumeRole response missing credentials"))?; + + let access_key_id = creds.access_key_id().to_string(); + let secret_access_key = creds.secret_access_key().to_string(); + let session_token = creds.session_token().to_string(); + + let now_ms = current_time_ms(); + let expires_at_ms = creds + .expiration() + .to_millis() + .unwrap_or_else(|_| now_ms + max_lifetime_i64 * 1000); + let max_expires = now_ms + max_lifetime_i64 * 1000; + let expires_at_ms = expires_at_ms.min(max_expires); + + let mut additional = HashMap::new(); + additional.insert("AWS_SECRET_ACCESS_KEY".to_string(), secret_access_key); + additional.insert("AWS_SESSION_TOKEN".to_string(), session_token); + + Ok(MintedCredential { + access_token: access_key_id, + expires_at_ms, + refresh_token: None, + additional_credentials: additional, + }) +} + async fn request_token( token_url: &str, form: &[(String, String)], @@ -603,6 +722,7 @@ async fn request_token( refresh_token: token .refresh_token .filter(|refresh_token| !refresh_token.trim().is_empty()), + additional_credentials: HashMap::new(), }) } @@ -1164,6 +1284,214 @@ mod tests { ); } + #[test] + fn refresh_strategy_name_includes_aws_sts() { + assert_eq!( + refresh_strategy_name(ProviderCredentialRefreshStrategy::AwsStsAssumeRole as i32), + "aws_sts_assume_role" + ); + } + + #[tokio::test] + async fn aws_sts_assume_role_mints_three_credentials_from_mock_endpoint() { + let mock_server = MockServer::start().await; + Mock::given(method("POST")) + .and(body_string_contains("Action=AssumeRole")) + .and(body_string_contains("RoleArn=arn")) + .respond_with(ResponseTemplate::new(200).set_body_string( + r#" + + + AROA3XFRBF23:test-session + arn:aws:sts::123456789012:assumed-role/TestRole/test-session + + + ASIAMOCKKEY + MockSecretAccessKey123 + MockSessionTokenXYZ + 2099-01-01T00:00:00Z + + + + 01234567-89ab-cdef-0123-456789abcdef + +"#, + )) + .mount(&mock_server) + .await; + + let store = test_store().await; + let prov = provider("aws-sts-test", "aws"); + store.put_message(&prov).await.unwrap(); + + let state = new_refresh_state( + &prov, + "AWS_ACCESS_KEY_ID", + NewRefreshStateConfig { + strategy: ProviderCredentialRefreshStrategy::AwsStsAssumeRole, + material: HashMap::from([ + ( + "role_arn".to_string(), + "arn:aws:iam::123456789012:role/TestRole".to_string(), + ), + ("session_name".to_string(), "test-session".to_string()), + ("aws_access_key_id".to_string(), "AKIATESTKEY".to_string()), + ( + "aws_secret_access_key".to_string(), + "TestSecretKey".to_string(), + ), + ("sts_endpoint_url".to_string(), mock_server.uri()), + ]), + secret_material_keys: vec!["aws_secret_access_key".to_string()], + expires_at_ms: 0, + token_url: String::new(), + scopes: Vec::new(), + refresh_before_seconds: 300, + max_lifetime_seconds: 3600, + }, + ) + .unwrap(); + put_refresh_state(&store, &state).await.unwrap(); + + let refreshed = refresh_provider_credential(&store, "aws-sts-test", "AWS_ACCESS_KEY_ID") + .await + .unwrap(); + assert_eq!(refreshed.status, "refreshed"); + assert!(refreshed.expires_at_ms > 0); + + let stored = store + .get_message_by_name::("aws-sts-test") + .await + .unwrap() + .unwrap(); + assert_eq!( + stored.credentials.get("AWS_ACCESS_KEY_ID"), + Some(&"ASIAMOCKKEY".to_string()) + ); + assert_eq!( + stored.credentials.get("AWS_SECRET_ACCESS_KEY"), + Some(&"MockSecretAccessKey123".to_string()) + ); + assert_eq!( + stored.credentials.get("AWS_SESSION_TOKEN"), + Some(&"MockSessionTokenXYZ".to_string()) + ); + } + + #[tokio::test] + async fn apply_minted_credential_writes_additional_keys() { + use super::apply_minted_credential; + + let store = test_store().await; + let mut prov = provider("aws-test", "aws"); + prov.credentials + .insert("AWS_ACCESS_KEY_ID".to_string(), "old-key".to_string()); + store.put_message(&prov).await.unwrap(); + + let minted = super::MintedCredential { + access_token: "AKIAIOSFODNN7EXAMPLE".to_string(), + expires_at_ms: 4_000_000_000_000, + refresh_token: None, + additional_credentials: HashMap::from([ + ( + "AWS_SECRET_ACCESS_KEY".to_string(), + "wJalrXUtnFEMI/K7MDENG/bPxRfiCYEXAMPLEKEY".to_string(), + ), + ( + "AWS_SESSION_TOKEN".to_string(), + "FwoGZXIvYXdzEBYaDH...EXAMPLETOKEN".to_string(), + ), + ]), + }; + + apply_minted_credential(&store, &prov, "AWS_ACCESS_KEY_ID", &minted) + .await + .unwrap(); + + let stored = store + .get_message_by_name::("aws-test") + .await + .unwrap() + .unwrap(); + assert_eq!( + stored.credentials.get("AWS_ACCESS_KEY_ID"), + Some(&"AKIAIOSFODNN7EXAMPLE".to_string()) + ); + assert_eq!( + stored.credentials.get("AWS_SECRET_ACCESS_KEY"), + Some(&"wJalrXUtnFEMI/K7MDENG/bPxRfiCYEXAMPLEKEY".to_string()) + ); + assert_eq!( + stored.credentials.get("AWS_SESSION_TOKEN"), + Some(&"FwoGZXIvYXdzEBYaDH...EXAMPLETOKEN".to_string()) + ); + assert_eq!( + stored.credential_expires_at_ms.get("AWS_ACCESS_KEY_ID"), + Some(&4_000_000_000_000) + ); + assert_eq!( + stored.credential_expires_at_ms.get("AWS_SECRET_ACCESS_KEY"), + Some(&4_000_000_000_000) + ); + assert_eq!( + stored.credential_expires_at_ms.get("AWS_SESSION_TOKEN"), + Some(&4_000_000_000_000) + ); + } + + #[tokio::test] + async fn apply_minted_credential_validates_additional_keys_against_sandboxes() { + use super::apply_minted_credential; + + let store = test_store().await; + let mut existing_provider = provider("existing-aws", "aws"); + existing_provider + .credentials + .insert("AWS_SECRET_ACCESS_KEY".to_string(), "existing".to_string()); + store.put_message(&existing_provider).await.unwrap(); + + let refreshing_provider = provider("refreshing-aws", "aws"); + store.put_message(&refreshing_provider).await.unwrap(); + + store + .put_message(&Sandbox { + metadata: Some(ObjectMeta { + id: "sandbox-aws-collision".to_string(), + name: "aws-collision".to_string(), + created_at_ms: 1, + labels: HashMap::new(), + resource_version: 0, + }), + spec: Some(SandboxSpec { + providers: vec!["existing-aws".to_string(), "refreshing-aws".to_string()], + ..SandboxSpec::default() + }), + ..Default::default() + }) + .await + .unwrap(); + + let minted = super::MintedCredential { + access_token: "AKIAIOSFODNN7EXAMPLE".to_string(), + expires_at_ms: 4_000_000_000_000, + refresh_token: None, + additional_credentials: HashMap::from([ + ( + "AWS_SECRET_ACCESS_KEY".to_string(), + "secret-key".to_string(), + ), + ("AWS_SESSION_TOKEN".to_string(), "session-token".to_string()), + ]), + }; + + let err = + apply_minted_credential(&store, &refreshing_provider, "AWS_ACCESS_KEY_ID", &minted) + .await + .unwrap_err(); + assert_eq!(err.code(), tonic::Code::FailedPrecondition); + assert!(err.message().contains("AWS_SECRET_ACCESS_KEY")); + } + fn provider(name: &str, provider_type: &str) -> Provider { Provider { metadata: Some(ObjectMeta { diff --git a/crates/openshell-supervisor-network/src/l7/mod.rs b/crates/openshell-supervisor-network/src/l7/mod.rs index d10c19b8e8..005e37ff6e 100644 --- a/crates/openshell-supervisor-network/src/l7/mod.rs +++ b/crates/openshell-supervisor-network/src/l7/mod.rs @@ -455,18 +455,26 @@ fn validate_host_wildcard(errors: &mut Vec, loc: &str, host: &str) { let labels: Vec<&str> = host.split('.').collect(); let first_label = labels.first().copied().unwrap_or_default(); - if labels.iter().skip(1).any(|label| label.contains('*')) { - errors.push(format!( - "{loc}: host wildcard may only appear in the first DNS label, got '{host}'" - )); - return; - } if first_label.contains("**") && first_label != "**" { errors.push(format!( "{loc}: recursive host wildcard '**' is only allowed as the entire first DNS label, got '{host}'" )); return; } + for label in labels.iter().skip(1).copied() { + if label.contains("**") { + errors.push(format!( + "{loc}: recursive host wildcard '**' is only allowed as the entire first DNS label, got '{host}'" + )); + return; + } + if label.contains('*') && label != "*" { + errors.push(format!( + "{loc}: middle DNS label wildcard must be the entire label '*', got '{host}'" + )); + return; + } + } // Reject TLD or single-label wildcards. They are accepted by the policy // engine but silently fail at the proxy layer (see #787). @@ -2877,12 +2885,36 @@ mod tests { } #[test] - fn validate_wildcard_host_mid_label_error() { + fn validate_wildcard_host_middle_label_star_valid_no_error() { + let data = serde_json::json!({ + "network_policies": { + "test": { + "endpoints": [{ + "host": "*.s3.*.amazonaws.com", + "port": 443 + }], + "binaries": [] + } + } + }); + let (errors, warnings) = validate_l7_policies(&data); + assert!( + errors.is_empty(), + "*.s3.*.amazonaws.com should be valid, got errors: {errors:?}" + ); + assert!( + warnings.is_empty(), + "*.s3.*.amazonaws.com should not warn, got warnings: {warnings:?}" + ); + } + + #[test] + fn validate_wildcard_host_middle_label_partial_star_error() { let data = serde_json::json!({ "network_policies": { "test": { "endpoints": [{ - "host": "foo.*.example.com", + "host": "*.s3.us-*.amazonaws.com", "port": 443 }], "binaries": [] @@ -2891,8 +2923,30 @@ mod tests { }); let (errors, _warnings) = validate_l7_policies(&data); assert!( - errors.iter().any(|e| e.contains("first DNS label")), - "Mid-label wildcard should be rejected, got errors: {errors:?}" + errors + .iter() + .any(|e| e.contains("middle DNS label wildcard")), + "Partial middle-label wildcard should be rejected, got errors: {errors:?}" + ); + } + + #[test] + fn validate_wildcard_host_middle_label_double_star_error() { + let data = serde_json::json!({ + "network_policies": { + "test": { + "endpoints": [{ + "host": "*.s3.**.amazonaws.com", + "port": 443 + }], + "binaries": [] + } + } + }); + let (errors, _warnings) = validate_l7_policies(&data); + assert!( + errors.iter().any(|e| e.contains("recursive host wildcard")), + "Middle-label ** wildcard should be rejected, got errors: {errors:?}" ); } diff --git a/crates/openshell-supervisor-network/src/l7/rest.rs b/crates/openshell-supervisor-network/src/l7/rest.rs index 5c62f6f9ee..99f2f328eb 100644 --- a/crates/openshell-supervisor-network/src/l7/rest.rs +++ b/crates/openshell-supervisor-network/src/l7/rest.rs @@ -560,11 +560,21 @@ where // route chunked requests to the streaming path, but // guard here as defense-in-depth. let body_length = parse_body_length(header_str)?; - if matches!(body_length, BodyLength::Chunked) { - return Err(miette!( - "SigV4 body signing requires Content-Length; \ - chunked transfer encoding is not supported in this mode" - )); + match body_length { + BodyLength::ContentLength(_) | BodyLength::None => { + // ContentLength: buffer and sign the body. + // None: no body (e.g. GET/HEAD/DELETE) — sign + // with the empty-body hash. boto3 sends + // x-amz-content-sha256 set to the SHA-256 of + // "" on all requests, which routes here via + // detect_payload_mode's catch-all. + } + BodyLength::Chunked => { + return Err(miette!( + "SigV4 body signing requires Content-Length; \ + chunked transfer encoding is not supported in this mode" + )); + } } // NOTE(defense-in-depth): Build the full request from // rewritten headers + body. `rewrite_result.rewritten` @@ -5402,6 +5412,83 @@ mod tests { assert!(!forwarded.contains("openshell:resolve:env:")); } + #[tokio::test] + async fn relay_sigv4_body_signing_signs_empty_body_without_content_length() { + let (_, resolver) = SecretResolver::from_provider_env( + [ + ("AWS_ACCESS_KEY_ID".to_string(), "AKIATESTKEY".to_string()), + ( + "AWS_SECRET_ACCESS_KEY".to_string(), + "test-secret-key".to_string(), + ), + ] + .into_iter() + .collect(), + ); + let resolver = resolver.expect("resolver"); + + let raw_header = + b"GET /?list-type=2 HTTP/1.1\r\nHost: s3.us-east-1.amazonaws.com\r\n\r\n".to_vec(); + let req = L7Request { + action: "GET".to_string(), + target: "/".to_string(), + query_params: HashMap::from([("list-type".to_string(), vec!["2".to_string()])]), + raw_header, + body_length: BodyLength::None, + }; + let (mut proxy_to_upstream, mut upstream_side) = tokio::io::duplex(8192); + let (mut _app_side, mut proxy_to_client) = tokio::io::duplex(8192); + + let upstream_task = tokio::spawn(async move { + let mut buf = vec![0u8; 8192]; + let mut total = 0usize; + loop { + let n = upstream_side.read(&mut buf[total..]).await.unwrap(); + if n == 0 { + break; + } + total += n; + if buf[..total].windows(4).any(|w| w == b"\r\n\r\n") { + upstream_side + .write_all(b"HTTP/1.1 200 OK\r\nContent-Length: 2\r\n\r\nok") + .await + .unwrap(); + upstream_side.flush().await.unwrap(); + break; + } + } + String::from_utf8_lossy(&buf[..total]).to_string() + }); + + let relay_result = tokio::time::timeout( + std::time::Duration::from_secs(5), + relay_http_request_with_options_guarded( + &req, + &mut proxy_to_client, + &mut proxy_to_upstream, + RelayRequestOptions { + resolver: Some(&resolver), + credential_signing: crate::l7::CredentialSigning::SigV4Body, + signing_service: "s3", + signing_region: "us-east-1", + host: "s3.us-east-1.amazonaws.com", + port: 443, + ..Default::default() + }, + ), + ) + .await + .expect("relay must not deadlock"); + drop(proxy_to_upstream); + let forwarded = upstream_task.await.expect("upstream task should complete"); + + relay_result.expect("bodyless GET with SigV4Body should succeed"); + assert!( + forwarded.contains("authorization:"), + "forwarded request should have SigV4 authorization header: {forwarded}" + ); + } + #[tokio::test] async fn relay_injects_url_path_credential_telegram_style() { let (child_env, resolver) = SecretResolver::from_provider_env( diff --git a/crates/openshell-supervisor-network/src/opa.rs b/crates/openshell-supervisor-network/src/opa.rs index 829c63caa4..935946920d 100644 --- a/crates/openshell-supervisor-network/src/opa.rs +++ b/crates/openshell-supervisor-network/src/opa.rs @@ -5804,6 +5804,88 @@ network_policies: ); } + #[test] + fn wildcard_host_middle_label_matches_one_region_label() { + let data = r#" +network_policies: + s3: + name: s3 + endpoints: + - { host: "*.s3.*.amazonaws.com", port: 443 } + binaries: + - { path: /usr/bin/curl } +"#; + let engine = OpaEngine::from_strings(TEST_POLICY, data).unwrap(); + let input = NetworkInput { + host: "my-bucket.s3.us-east-1.amazonaws.com".into(), + port: 443, + binary_path: PathBuf::from("/usr/bin/curl"), + binary_sha256: "unused".into(), + ancestors: vec![], + cmdline_paths: vec![], + }; + let decision = engine.evaluate_network(&input).unwrap(); + assert!( + decision.allowed, + "*.s3.*.amazonaws.com should match one regional S3 label: {}", + decision.reason + ); + } + + #[test] + fn wildcard_host_middle_label_does_not_match_missing_bucket_label() { + let data = r#" +network_policies: + s3: + name: s3 + endpoints: + - { host: "*.s3.*.amazonaws.com", port: 443 } + binaries: + - { path: /usr/bin/curl } +"#; + let engine = OpaEngine::from_strings(TEST_POLICY, data).unwrap(); + let input = NetworkInput { + host: "s3.us-east-1.amazonaws.com".into(), + port: 443, + binary_path: PathBuf::from("/usr/bin/curl"), + binary_sha256: "unused".into(), + ancestors: vec![], + cmdline_paths: vec![], + }; + let decision = engine.evaluate_network(&input).unwrap(); + assert!( + !decision.allowed, + "*.s3.*.amazonaws.com should not match path-style S3 host" + ); + } + + #[test] + fn wildcard_host_middle_label_does_not_skip_dualstack_label() { + let data = r#" +network_policies: + s3: + name: s3 + endpoints: + - { host: "*.s3.*.amazonaws.com", port: 443 } + binaries: + - { path: /usr/bin/curl } +"#; + let engine = OpaEngine::from_strings(TEST_POLICY, data).unwrap(); + let input = NetworkInput { + host: "my-bucket.s3.dualstack.us-east-1.amazonaws.com".into(), + port: 443, + binary_path: PathBuf::from("/usr/bin/curl"), + binary_sha256: "unused".into(), + ancestors: vec![], + cmdline_paths: vec![], + }; + let decision = engine.evaluate_network(&input).unwrap(); + assert!( + !decision.allowed, + "*.s3.*.amazonaws.com should not match the extra dualstack label" + ); + } + #[test] fn wildcard_host_multi_port() { let data = r#" diff --git a/crates/openshell-tui/src/app.rs b/crates/openshell-tui/src/app.rs index 4538b207d6..ae37a52a33 100644 --- a/crates/openshell-tui/src/app.rs +++ b/crates/openshell-tui/src/app.rs @@ -744,6 +744,9 @@ fn refresh_strategy_label(strategy: i32) -> &'static str { openshell_core::proto::ProviderCredentialRefreshStrategy::GoogleServiceAccountJwt => { "google_service_account_jwt" } + openshell_core::proto::ProviderCredentialRefreshStrategy::AwsStsAssumeRole => { + "aws_sts_assume_role" + } openshell_core::proto::ProviderCredentialRefreshStrategy::Unspecified => "unspecified", } } diff --git a/docs/sandboxes/manage-providers.mdx b/docs/sandboxes/manage-providers.mdx index 1324859ed7..98e706be6e 100644 --- a/docs/sandboxes/manage-providers.mdx +++ b/docs/sandboxes/manage-providers.mdx @@ -135,9 +135,9 @@ Use `0` as the timestamp to clear expiry for a credential key. Provider refresh stores non-injectable refresh material separately from the provider's current credential values. The gateway can mint OAuth2 refresh-token -tokens, OAuth2 client credentials tokens, and Google service account JWT tokens, -then write the current access token back to the provider record for sandbox -injection. +tokens, OAuth2 client credentials tokens, Google service account JWT tokens, and +AWS STS temporary credentials, then write the current access token back to the +provider record for sandbox injection. Configure refresh metadata for one injectable credential key: @@ -177,6 +177,46 @@ Force a gateway-managed refresh for one credential: openshell provider refresh rotate my-graph --credential-key MS_GRAPH_ACCESS_TOKEN ``` +### AWS STS + +AWS STS refresh requires `providers_v2_enabled=true`. The gateway calls +`sts:AssumeRole` and writes three short-lived credentials (`AWS_ACCESS_KEY_ID`, +`AWS_SECRET_ACCESS_KEY`, `AWS_SESSION_TOKEN`) to the provider record atomically. + +```shell +openshell settings set --global --key providers_v2_enabled --value true --yes + +openshell provider create --name my-aws --type aws-s3 + +openshell provider refresh configure my-aws \ + --credential-key AWS_ACCESS_KEY_ID \ + --strategy aws-sts-assume-role \ + --material role_arn="arn:aws:iam::123456789012:role/SandboxS3Writer" \ + --material session_name="openshell-sandbox" + +openshell provider refresh rotate my-aws --credential-key AWS_ACCESS_KEY_ID +``` + +The gateway resolves its own AWS credentials using the default credential chain +(instance role, IRSA, ECS task role, environment variables). For gateways not +running on AWS, provide explicit IAM keys via refresh material: + +```shell +openshell provider refresh configure my-aws \ + --credential-key AWS_ACCESS_KEY_ID \ + --strategy aws-sts-assume-role \ + --material role_arn="arn:aws:iam::123456789012:role/SandboxS3Writer" \ + --material aws_access_key_id="$AWS_ACCESS_KEY_ID" \ + --material aws_secret_access_key="$AWS_SECRET_ACCESS_KEY" \ + --secret-material-key aws_secret_access_key +``` + +Use the generic `aws` profile type for multi-service access and scope endpoints +via sandbox network policy. Use `aws-s3` for S3-specific endpoint rules. + +The proxy re-signs requests using SigV4 before forwarding to AWS. Both curl and +Python boto3 are supported. + External refresh systems should continue to push new current credentials through `openshell provider update`. The `--credential-expires-at` option works for static credentials, externally refreshed credentials, and gateway-managed diff --git a/examples/aws-s3-sts.md b/examples/aws-s3-sts.md new file mode 100644 index 0000000000..26ae035942 --- /dev/null +++ b/examples/aws-s3-sts.md @@ -0,0 +1,269 @@ +# Manual E2E Test: S3 Access via STS Credentials + +This guide walks through an end-to-end test of an OpenShell sandbox accessing +AWS S3 using gateway-minted STS temporary credentials with proxy-side SigV4 +re-signing. The sandbox never sees real AWS credentials — the proxy resolves +placeholders and signs requests on the fly. + +## Prerequisites + +- AWS CLI authenticated (`aws sts get-caller-identity` succeeds) +- Podman running (`podman info` succeeds) +- OpenShell built from source with AWS STS refresh and SigV4 signing support + +## 1. Create AWS test resources + +Create an S3 bucket and an IAM role the gateway can assume: + +```shell +BUCKET="openshell-sts-test-$(date +%s)" +ACCOUNT=$(aws sts get-caller-identity --query Account --output text) + +aws s3 mb "s3://${BUCKET}" --region us-east-1 + +aws iam create-role \ + --role-name openshell-sts-test-role \ + --assume-role-policy-document '{ + "Version": "2012-10-17", + "Statement": [{ + "Effect": "Allow", + "Principal": {"AWS": "arn:aws:iam::'${ACCOUNT}':root"}, + "Action": "sts:AssumeRole" + }] + }' + +aws iam put-role-policy \ + --role-name openshell-sts-test-role \ + --policy-name s3-access \ + --policy-document '{ + "Version": "2012-10-17", + "Statement": [{ + "Effect": "Allow", + "Action": ["s3:PutObject", "s3:GetObject", "s3:ListBucket"], + "Resource": ["arn:aws:s3:::'${BUCKET}'", "arn:aws:s3:::'${BUCKET}'/*"] + }] + }' +``` + +Verify the role works: + +```shell +aws sts assume-role \ + --role-arn "arn:aws:iam::${ACCOUNT}:role/openshell-sts-test-role" \ + --role-session-name test \ + --query Credentials.AccessKeyId --output text +``` + +## 2. Build the supervisor image + +The supervisor image must include the SigV4 re-signing code and the updated +proto definitions. Build it from the branch: + +```shell +CONTAINER_ENGINE=podman IMAGE_TAG=dev mise run build:docker:supervisor +``` + +Verify the image exists locally: + +```shell +podman images | grep "openshell/supervisor.*dev" +``` + +## 3. Start the gateway + +The gateway needs AWS credentials in its environment to call `sts:AssumeRole`. +Export them before starting: + +```shell +eval "$(aws configure export-credentials --format env)" +``` + +The gateway must use the locally built supervisor image. The `mise run gateway` +script places `supervisor_image` in the wrong TOML section, so start the gateway +binary directly with a hand-written config. + +Write `.cache/gateway-podman/gateway.toml` in the repo root (adjust JWT paths +if your gateway cache directory differs): + +```toml +[openshell] +version = 1 + +[openshell.gateway] +compute_drivers = ["podman"] +default_image = "ghcr.io/nvidia/openshell-community/sandboxes/base:latest" +disable_tls = true +supervisor_image = "localhost/openshell/supervisor:dev" + +[openshell.gateway.auth] +allow_unauthenticated_users = true + +[openshell.gateway.gateway_jwt] +signing_key_path = ".cache/gateway-podman/tls/jwt/signing.pem" +public_key_path = ".cache/gateway-podman/tls/jwt/public.pem" +kid_path = ".cache/gateway-podman/tls/jwt/kid" +gateway_id = "podman-dev" +ttl_secs = 3600 + +[openshell.drivers.podman] +image_pull_policy = "missing" +``` + +If the JWT key files do not exist yet, run `mise run gateway` once to generate +them, then stop the gateway and restart with the config above. + +Start the gateway: + +```shell +eval "$(aws configure export-credentials --format env)" +./target/debug/openshell-gateway \ + --config .cache/gateway-podman/gateway.toml \ + --port 18080 --log-level info --drivers podman --disable-tls \ + --db-url "sqlite:.cache/gateway-podman/gateway.db?mode=rwc" +``` + +## 4. Configure the provider + +In a separate terminal: + +```shell +export OPENSHELL_BASE_URL=http://localhost:18080 + +# Enable provider v2 (required for STS) +openshell settings set --global --key providers_v2_enabled --value true --yes + +# Create the provider with the aws-s3 profile +openshell provider create --name s3-test --type aws-s3 \ + --credential AWS_ACCESS_KEY_ID=placeholder + +# Configure STS refresh +openshell provider refresh configure s3-test \ + --credential-key AWS_ACCESS_KEY_ID \ + --strategy aws-sts-assume-role \ + --material role_arn="arn:aws:iam::${ACCOUNT}:role/openshell-sts-test-role" \ + --material session_name="openshell-sandbox" \ + --material aws_region="us-east-1" + +# Mint the first set of credentials +openshell provider refresh rotate s3-test \ + --credential-key AWS_ACCESS_KEY_ID + +# Verify +openshell provider refresh status s3-test +``` + +The status should show `refreshed` with an expiry ~1 hour from now. + +## 5. Test S3 access from a sandbox + +### Using boto3 (Python) + +```shell +openshell sandbox create --name s3-smoke \ + --provider s3-test \ + -- bash -c ' +export AWS_CA_BUNDLE=/etc/openshell-tls/ca-bundle.pem +pip install boto3 -q 2>&1 | tail -1 +python3 -c " +import boto3 + +s3 = boto3.client(\"s3\", region_name=\"us-east-1\") + +print(\"Upload...\") +s3.put_object( + Bucket=\"'"${BUCKET}"'\", + Key=\"from-sandbox.txt\", + Body=b\"hello from openshell sandbox via STS\" +) +print(\"OK\") + +print(\"List...\") +resp = s3.list_objects_v2(Bucket=\"'"${BUCKET}"'\", MaxKeys=5) +for obj in resp.get(\"Contents\", []): + print(\" \" + obj[\"Key\"] + \" (\" + str(obj[\"Size\"]) + \" bytes)\") + +print(\"Download...\") +body = s3.get_object( + Bucket=\"'"${BUCKET}"'\", + Key=\"from-sandbox.txt\" +)[\"Body\"].read() +print(body.decode()) +" +' +``` + +All three operations should succeed. The download should print +`hello from openshell sandbox via STS`. + +### Using curl + +```shell +openshell sandbox create --name s3-curl \ + --provider s3-test \ + -- bash -c ' +BUCKET="'"${BUCKET}"'" +REGION="us-east-1" +CA=/etc/openshell-tls/ca-bundle.pem + +echo "=== Upload ===" +curl -s --cacert $CA -X PUT -H "Content-Type: text/plain" \ + -d "hello from openshell sandbox via STS" \ + "https://${BUCKET}.s3.${REGION}.amazonaws.com/from-sandbox.txt" \ + -w "HTTP %{http_code}\n" + +echo "" +echo "=== Download ===" +curl -s --cacert $CA \ + "https://${BUCKET}.s3.${REGION}.amazonaws.com/from-sandbox.txt" \ + -w "\nHTTP %{http_code}\n" +' +``` + +Both operations should return `HTTP 200`. + +### What's happening + +1. The gateway called `sts:AssumeRole` and stored three short-lived credentials + (`AWS_ACCESS_KEY_ID`, `AWS_SECRET_ACCESS_KEY`, `AWS_SESSION_TOKEN`) in the + provider record. +2. The sandbox received placeholder values for these credentials as environment + variables. +3. The client (boto3 or curl) sent an HTTP request through the sandbox proxy's + CONNECT tunnel. boto3 signs the request with placeholder credentials; curl + sends unsigned requests. Either way, the proxy handles it. +4. The proxy terminated TLS, stripped any existing AWS auth headers, resolved + the real credentials from the `SecretResolver`, computed a fresh SigV4 + signature using the `aws-sigv4` crate, and forwarded the signed request to + S3. +5. S3 validated the signature and accepted the request. + +The sandbox never saw real AWS credentials — only placeholders. + +### TLS CA trust + +The proxy terminates TLS and presents a certificate signed by the OpenShell +Sandbox CA. Curl needs `--cacert /etc/openshell-tls/ca-bundle.pem` to trust +it. Python clients need `AWS_CA_BUNDLE=/etc/openshell-tls/ca-bundle.pem` set +in the environment. + +## 6. Clean up + +```shell +# Delete the sandbox +openshell sandbox delete s3-smoke + +# Delete AWS resources +aws s3 rb "s3://${BUCKET}" --force +aws iam delete-role-policy --role-name openshell-sts-test-role --policy-name s3-access +aws iam delete-role --role-name openshell-sts-test-role +``` + +## Troubleshooting + +| Symptom | Cause | Fix | +|---------|-------|-----| +| `STS AssumeRole failed: dispatch failure` | Gateway doesn't have AWS credentials | Export credentials before starting: `eval "$(aws configure export-credentials --format env)"` | +| `Policy discovery sync failed: invalid wire type` | Supervisor image doesn't have updated proto | Rebuild: `CONTAINER_ENGINE=podman IMAGE_TAG=dev mise run build:docker:supervisor` | +| `CONNECT ... not permitted by policy` | Binary not in profile's `binaries` list | Use curl (in the list) or add your binary path to the policy | +| `403 AccessDenied` from S3 | IAM role missing permissions, or STS creds expired | Check `openshell provider refresh status`; re-rotate if expired | +| Supervisor uses wrong image | `mise run gateway` places `supervisor_image` in wrong TOML section | Use the hand-written config from step 3 instead of `mise run gateway` | diff --git a/proto/openshell.proto b/proto/openshell.proto index d2d884f2e8..bf49d2b391 100644 --- a/proto/openshell.proto +++ b/proto/openshell.proto @@ -974,6 +974,7 @@ enum ProviderCredentialRefreshStrategy { PROVIDER_CREDENTIAL_REFRESH_STRATEGY_OAUTH2_REFRESH_TOKEN = 3; PROVIDER_CREDENTIAL_REFRESH_STRATEGY_OAUTH2_CLIENT_CREDENTIALS = 4; PROVIDER_CREDENTIAL_REFRESH_STRATEGY_GOOGLE_SERVICE_ACCOUNT_JWT = 5; + PROVIDER_CREDENTIAL_REFRESH_STRATEGY_AWS_STS_ASSUME_ROLE = 6; } message ProviderCredentialRefreshMaterial { diff --git a/providers/aws-s3.yaml b/providers/aws-s3.yaml new file mode 100644 index 0000000000..87d5350e14 --- /dev/null +++ b/providers/aws-s3.yaml @@ -0,0 +1,112 @@ +# SPDX-FileCopyrightText: Copyright (c) 2025-2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. +# SPDX-License-Identifier: Apache-2.0 + +id: aws-s3 +display_name: AWS S3 +description: AWS S3 storage via STS temporary credentials +category: other +credentials: + - name: access_key_id + description: AWS access key ID (gateway-minted via STS) + env_vars: [AWS_ACCESS_KEY_ID] + required: true + refresh: + strategy: aws_sts_assume_role + refresh_before_seconds: 300 + max_lifetime_seconds: 3600 + material: + - name: role_arn + description: ARN of the IAM role to assume + required: true + secret: false + - name: session_name + description: Session name for CloudTrail attribution + required: false + secret: false + - name: external_id + description: External ID for cross-account role assumption + required: false + secret: false + - name: aws_region + description: AWS region for STS endpoint + required: false + secret: false + - name: aws_access_key_id + description: Long-lived IAM access key (only needed if gateway lacks ambient AWS credentials) + required: false + secret: false + - name: aws_secret_access_key + description: Long-lived IAM secret key (only needed if gateway lacks ambient AWS credentials) + required: false + secret: true + - name: secret_access_key + description: AWS secret access key (co-managed with access_key_id) + env_vars: [AWS_SECRET_ACCESS_KEY] + required: true + - name: session_token + description: AWS session token (co-managed with access_key_id) + env_vars: [AWS_SESSION_TOKEN] + required: true +endpoints: + # S3 regional endpoints. `*` matches exactly one DNS label, so the region + # wildcard does not cover extra labels like dualstack unless listed. + # us-east-1 uses the global endpoint without a region label. + - host: "*.s3.amazonaws.com" + port: 443 + protocol: rest + tls: terminate + access: read-write + enforcement: enforce + credential_signing: sigv4 + signing_service: s3 + signing_region: us-east-1 + - host: "s3.amazonaws.com" + port: 443 + protocol: rest + tls: terminate + access: read-write + enforcement: enforce + credential_signing: sigv4 + signing_service: s3 + signing_region: us-east-1 + - host: "*.s3.*.amazonaws.com" + port: 443 + protocol: rest + tls: terminate + access: read-write + enforcement: enforce + credential_signing: sigv4 + signing_service: s3 + - host: "s3.*.amazonaws.com" + port: 443 + protocol: rest + tls: terminate + access: read-write + enforcement: enforce + credential_signing: sigv4 + signing_service: s3 + - host: "*.s3.dualstack.*.amazonaws.com" + port: 443 + protocol: rest + tls: terminate + access: read-write + enforcement: enforce + credential_signing: sigv4 + signing_service: s3 + - host: "s3.dualstack.*.amazonaws.com" + port: 443 + protocol: rest + tls: terminate + access: read-write + enforcement: enforce + credential_signing: sigv4 + signing_service: s3 +binaries: + - /sandbox/.venv/bin/python + - /sandbox/.venv/bin/python3 + - /sandbox/.uv/python/** + - /usr/bin/python3 + - /usr/bin/python3.* + - /usr/bin/curl + - /usr/local/bin/aws + - /bin/bash diff --git a/providers/aws.yaml b/providers/aws.yaml new file mode 100644 index 0000000000..41323d204d --- /dev/null +++ b/providers/aws.yaml @@ -0,0 +1,49 @@ +# SPDX-FileCopyrightText: Copyright (c) 2025-2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. +# SPDX-License-Identifier: Apache-2.0 + +id: aws +display_name: AWS +description: Generic AWS access via STS temporary credentials +category: other +credentials: + - name: access_key_id + description: AWS access key ID (gateway-minted via STS) + env_vars: [AWS_ACCESS_KEY_ID] + required: true + refresh: + strategy: aws_sts_assume_role + refresh_before_seconds: 300 + max_lifetime_seconds: 3600 + material: + - name: role_arn + description: ARN of the IAM role to assume + required: true + secret: false + - name: session_name + description: Session name for CloudTrail attribution + required: false + secret: false + - name: external_id + description: External ID for cross-account role assumption + required: false + secret: false + - name: aws_region + description: AWS region for STS endpoint + required: false + secret: false + - name: aws_access_key_id + description: Long-lived IAM access key (only needed if gateway lacks ambient AWS credentials) + required: false + secret: false + - name: aws_secret_access_key + description: Long-lived IAM secret key (only needed if gateway lacks ambient AWS credentials) + required: false + secret: true + - name: secret_access_key + description: AWS secret access key (co-managed with access_key_id) + env_vars: [AWS_SECRET_ACCESS_KEY] + required: true + - name: session_token + description: AWS session token (co-managed with access_key_id) + env_vars: [AWS_SESSION_TOKEN] + required: true From 2c6dfb8022272983f8ff1223bcd94cd6f1b94f8e Mon Sep 17 00:00:00 2001 From: Russell Bryant Date: Wed, 8 Jul 2026 17:35:18 -0400 Subject: [PATCH 02/16] feat(providers): declarative multi-output credential refresh AWS STS AssumeRole mints three credentials from one refresh, but the model could only represent one; the extra AWS keys were hardcoded across the server. Add `additional_outputs` to the refresh profile so a refresh declares the sibling credentials it co-mints, resolve that mapping to concrete env keys, pin it in the refresh state, and drive minting, collision reservation, and env-key surfacing from it. - Consolidate the gateway-mintable strategy check into one shared function that includes AwsStsAssumeRole, fixing runtime resolvability so aws/aws-s3 providers can be created with --runtime-credentials. - Reserve all co-minted keys at configure time (resolved from the profile) instead of a hardcoded AWS list, so a configured-but-not-yet-minted refresh cannot collide at mint time. - Validate additional_outputs: known/required outputs per strategy, unique ids, existing single-env-var siblings, no self-refresh, standard AWS env keys, and pinned primary key. - Update aws/aws-s3 profiles and provider docs. Refs #1576 Signed-off-by: Russell Bryant --- architecture/gateway.md | 5 +- crates/openshell-providers/src/lib.rs | 6 +- crates/openshell-providers/src/profiles.rs | 625 +++++++++++++++++- crates/openshell-server/src/grpc/provider.rs | 201 +++++- .../openshell-server/src/provider_refresh.rs | 125 +++- docs/providers/aws-sigv4.mdx | 6 + docs/sandboxes/manage-providers.mdx | 6 +- docs/sandboxes/providers-v2.mdx | 30 +- examples/aws-s3-sts.md | 6 +- proto/openshell.proto | 15 + providers/aws-s3.yaml | 8 + providers/aws.yaml | 8 + 12 files changed, 997 insertions(+), 44 deletions(-) diff --git a/architecture/gateway.md b/architecture/gateway.md index d873b2a105..9f3991c667 100644 --- a/architecture/gateway.md +++ b/architecture/gateway.md @@ -164,7 +164,10 @@ state, SSH sessions, policy revisions, settings, inference configuration, and deployment records. Provider refresh material is stored as a separate object scoped to the provider instance through `objects.scope`; the provider record keeps only the current injectable credential values and optional per-credential -expiry timestamps. +expiry timestamps. A refresh normally mints one credential, but a strategy may +co-mint several (AWS STS mints the access key, secret key, and session token in +one call); the refresh state pins the resolved set of env keys it owns so +collision checks reserve all of them before the first mint. ### Optimistic Concurrency (CAS) diff --git a/crates/openshell-providers/src/lib.rs b/crates/openshell-providers/src/lib.rs index b15525e372..45408ea31e 100644 --- a/crates/openshell-providers/src/lib.rs +++ b/crates/openshell-providers/src/lib.rs @@ -19,9 +19,9 @@ pub use context::{DiscoveryContext, RealDiscoveryContext}; pub use discovery::{discover_from_profile, discover_with_spec}; pub use profiles::{ CredentialRefreshProfile, ProfileError, ProfileValidationDiagnostic, ProviderTypeProfile, - default_profiles, get_default_profile, normalize_profile_id, parse_profile_json, - parse_profile_yaml, profile_to_json, profile_to_yaml, profiles_to_json, profiles_to_yaml, - validate_profile_set, + default_profiles, get_default_profile, is_gateway_mintable_strategy, normalize_profile_id, + parse_profile_json, parse_profile_yaml, profile_to_json, profile_to_yaml, profiles_to_json, + profiles_to_yaml, validate_profile_set, }; #[derive(Debug, thiserror::Error)] diff --git a/crates/openshell-providers/src/profiles.rs b/crates/openshell-providers/src/profiles.rs index 4e03cdef9d..ee8b2be4bf 100644 --- a/crates/openshell-providers/src/profiles.rs +++ b/crates/openshell-providers/src/profiles.rs @@ -8,8 +8,9 @@ use openshell_core::proto::{ GraphqlOperation, L7Allow, L7DenyRule, L7QueryMatcher, L7Rule, McpOptions, NetworkBinary, NetworkEndpoint, NetworkPolicyRule, ProviderCredentialRefresh, - ProviderCredentialRefreshMaterial, ProviderCredentialRefreshStrategy, ProviderProfile, - ProviderProfileCategory, ProviderProfileCredential, ProviderProfileDiscovery, + ProviderCredentialRefreshMaterial, ProviderCredentialRefreshOutput, + ProviderCredentialRefreshStrategy, ProviderProfile, ProviderProfileCategory, + ProviderProfileCredential, ProviderProfileDiscovery, }; use openshell_core::secrets::uses_reserved_revision_namespace; use serde::ser::SerializeStruct; @@ -149,6 +150,11 @@ pub struct CredentialRefreshProfile { pub max_lifetime_seconds: i64, #[serde(default, skip_serializing_if = "Vec::is_empty")] pub material: Vec, + /// Additional credentials this refresh mints beyond its primary credential. + /// Each entry maps a strategy-defined semantic output id to a sibling + /// credential whose `env_vars` receive the minted value. + #[serde(default, skip_serializing_if = "Vec::is_empty")] + pub additional_outputs: Vec, } #[derive(Debug, Clone, Deserialize, Serialize, PartialEq, Eq)] @@ -162,6 +168,14 @@ pub struct CredentialRefreshMaterialProfile { pub secret: bool, } +#[derive(Debug, Clone, Deserialize, Serialize, PartialEq, Eq)] +pub struct CredentialRefreshOutputProfile { + /// Strategy-defined semantic output id (e.g. `session_token`). + pub output: String, + /// Sibling credential name whose `env_vars` receive this output. + pub credential: String, +} + #[derive(Debug, Clone, Default, Deserialize, Serialize, PartialEq, Eq)] pub struct DiscoveryProfile { #[serde(default, skip_serializing_if = "Vec::is_empty")] @@ -382,12 +396,15 @@ impl ProviderTypeProfile { /// Empty provider creation is allowed when at least one credential can be /// resolved at runtime, and every required credential can be resolved at /// runtime. Runtime-resolvable credentials are either gateway-mintable - /// refresh credentials or sandbox-side dynamic token grants. + /// refresh credentials, sandbox-side dynamic token grants, or additional + /// outputs co-minted by another credential's gateway-mintable refresh. #[must_use] pub fn allows_empty_provider_credentials(&self) -> bool { + let co_minted = self.co_minted_credential_names(); let mut has_runtime_resolvable_credential = false; for credential in &self.credentials { - let is_runtime_resolvable = credential.is_runtime_resolvable(); + let is_runtime_resolvable = + credential.is_runtime_resolvable() || co_minted.contains(credential.name.as_str()); if credential.required && !is_runtime_resolvable { return false; } @@ -396,6 +413,54 @@ impl ProviderTypeProfile { has_runtime_resolvable_credential } + /// Names of credentials produced as `additional_outputs` of a + /// gateway-mintable refresh on some other credential. + fn co_minted_credential_names(&self) -> HashSet<&str> { + self.credentials + .iter() + .filter_map(|credential| credential.refresh.as_ref()) + .filter(|refresh| refresh.is_gateway_mintable()) + .flat_map(|refresh| refresh.additional_outputs.iter()) + .map(|output| output.credential.as_str()) + .collect() + } + + /// For the credential resolved by `credential_key` (matched by name or env + /// var) that carries a refresh with `additional_outputs`, resolve each + /// output to the concrete env key of its target credential. Returns a map + /// of semantic output id -> env key (empty when there is no such refresh or + /// no additional outputs). Skips outputs whose target credential is missing + /// or does not declare exactly one env var; `validate_profile_set` reports + /// those as errors. + #[must_use] + pub fn resolved_additional_output_keys(&self, credential_key: &str) -> HashMap { + let Some(refresh) = self + .credentials + .iter() + .find(|credential| { + credential.name == credential_key + || credential.env_vars.iter().any(|env| env == credential_key) + }) + .and_then(|credential| credential.refresh.as_ref()) + else { + return HashMap::new(); + }; + refresh + .additional_outputs + .iter() + .filter_map(|output| { + let target = self + .credentials + .iter() + .find(|credential| credential.name == output.credential)?; + let [env_key] = target.env_vars.as_slice() else { + return None; + }; + Some((output.output.clone(), env_key.clone())) + }) + .collect() + } + /// Returns the credential suitable for `--from-gcloud-adc` bootstrap, if any. /// /// A credential qualifies when its refresh strategy is `Oauth2RefreshToken` @@ -476,12 +541,70 @@ impl CredentialProfile { impl CredentialRefreshProfile { #[must_use] pub fn is_gateway_mintable(&self) -> bool { - matches!( - self.strategy, - ProviderCredentialRefreshStrategy::Oauth2RefreshToken - | ProviderCredentialRefreshStrategy::Oauth2ClientCredentials - | ProviderCredentialRefreshStrategy::GoogleServiceAccountJwt - ) + is_gateway_mintable_strategy(self.strategy) + } +} + +/// Whether the gateway can mint credentials for this refresh strategy (as +/// opposed to strategies resolved elsewhere, e.g. `Static`/`External`). +/// +/// Single source of truth shared with `openshell-server`; keep in sync with the +/// dispatch in `provider_refresh::mint_credential`. +#[must_use] +pub fn is_gateway_mintable_strategy(strategy: ProviderCredentialRefreshStrategy) -> bool { + matches!( + strategy, + ProviderCredentialRefreshStrategy::Oauth2RefreshToken + | ProviderCredentialRefreshStrategy::Oauth2ClientCredentials + | ProviderCredentialRefreshStrategy::GoogleServiceAccountJwt + | ProviderCredentialRefreshStrategy::AwsStsAssumeRole + ) +} + +/// Semantic output ids a refresh strategy can produce **in addition** to its +/// primary credential, split into required and optional. Drives +/// `additional_outputs` validation. Strategies not listed here produce only a +/// single (primary) credential and reject any `additional_outputs`. +#[must_use] +pub fn strategy_output_spec( + strategy: ProviderCredentialRefreshStrategy, +) -> (&'static [&'static str], &'static [&'static str]) { + match strategy { + ProviderCredentialRefreshStrategy::AwsStsAssumeRole => { + (&["secret_access_key", "session_token"], &[]) + } + _ => (&[], &[]), + } +} + +/// Expected primary env key for strategies that constrain it, or `None` when the +/// strategy accepts any caller-chosen primary key. AWS `SigV4` signing looks up +/// `AWS_ACCESS_KEY_ID` by name, so STS pins the primary key. +#[must_use] +pub fn strategy_primary_env_key( + strategy: ProviderCredentialRefreshStrategy, +) -> Option<&'static str> { + match strategy { + ProviderCredentialRefreshStrategy::AwsStsAssumeRole => Some("AWS_ACCESS_KEY_ID"), + _ => None, + } +} + +/// Expected concrete env key for a given strategy output, when the strategy +/// constrains it (the `SigV4` signer resolves these by name). +#[must_use] +pub fn strategy_output_env_key( + strategy: ProviderCredentialRefreshStrategy, + output: &str, +) -> Option<&'static str> { + match (strategy, output) { + (ProviderCredentialRefreshStrategy::AwsStsAssumeRole, "secret_access_key") => { + Some("AWS_SECRET_ACCESS_KEY") + } + (ProviderCredentialRefreshStrategy::AwsStsAssumeRole, "session_token") => { + Some("AWS_SESSION_TOKEN") + } + _ => None, } } @@ -679,6 +802,14 @@ fn credential_refresh_from_proto(refresh: &ProviderCredentialRefresh) -> Credent secret: material.secret, }) .collect(), + additional_outputs: refresh + .additional_outputs + .iter() + .map(|output| CredentialRefreshOutputProfile { + output: output.output.clone(), + credential: output.credential.clone(), + }) + .collect(), } } @@ -699,6 +830,14 @@ fn credential_refresh_to_proto(refresh: &CredentialRefreshProfile) -> ProviderCr secret: material.secret, }) .collect(), + additional_outputs: refresh + .additional_outputs + .iter() + .map(|output| ProviderCredentialRefreshOutput { + output: output.output.clone(), + credential: output.credential.clone(), + }) + .collect(), } } @@ -1291,6 +1430,136 @@ pub fn validate_profile_set( )); } } + + let strategy_name = provider_refresh_strategy_to_yaml(refresh.strategy); + let (required_outputs, optional_outputs) = strategy_output_spec(refresh.strategy); + let known_outputs: HashSet<&str> = required_outputs + .iter() + .chain(optional_outputs.iter()) + .copied() + .collect(); + + if known_outputs.is_empty() && !refresh.additional_outputs.is_empty() { + diagnostics.push(ProfileValidationDiagnostic::error( + source, + profile_id, + "credentials.refresh.additional_outputs", + format!("strategy {strategy_name} does not support additional_outputs"), + )); + } + + let mut seen_outputs = HashSet::new(); + let mut mapped_required = HashSet::new(); + for output in &refresh.additional_outputs { + let output_id = output.output.trim(); + let cred_name = output.credential.trim(); + + if output_id.is_empty() { + diagnostics.push(ProfileValidationDiagnostic::error( + source, + profile_id, + "credentials.refresh.additional_outputs.output", + "refresh additional output id is required", + )); + continue; + } + if !seen_outputs.insert(output_id.to_string()) { + diagnostics.push(ProfileValidationDiagnostic::error( + source, + profile_id, + "credentials.refresh.additional_outputs.output", + format!("duplicate refresh additional output: {output_id}"), + )); + } + if known_outputs.contains(output_id) { + if required_outputs.contains(&output_id) { + mapped_required.insert(output_id.to_string()); + } + } else if !known_outputs.is_empty() { + diagnostics.push(ProfileValidationDiagnostic::error( + source, + profile_id, + "credentials.refresh.additional_outputs.output", + format!( + "strategy {strategy_name} does not produce output '{output_id}'" + ), + )); + } + + let Some(sibling) = profile.credentials.iter().find(|c| c.name == cred_name) + else { + diagnostics.push(ProfileValidationDiagnostic::error( + source, + profile_id, + "credentials.refresh.additional_outputs.credential", + format!( + "refresh additional output '{output_id}' references unknown credential '{cred_name}'" + ), + )); + continue; + }; + if sibling.refresh.is_some() { + diagnostics.push(ProfileValidationDiagnostic::error( + source, + profile_id, + "credentials.refresh.additional_outputs.credential", + format!( + "credential '{cred_name}' is a refresh output and must not declare its own refresh" + ), + )); + } + if sibling.env_vars.len() != 1 { + diagnostics.push(ProfileValidationDiagnostic::error( + source, + profile_id, + "credentials.refresh.additional_outputs.credential", + format!( + "refresh output credential '{cred_name}' must declare exactly one env var" + ), + )); + continue; + } + let resolved = sibling.env_vars[0].as_str(); + if let Some(expected) = strategy_output_env_key(refresh.strategy, output_id) + && resolved != expected + { + diagnostics.push(ProfileValidationDiagnostic::error( + source, + profile_id, + "credentials.refresh.additional_outputs.credential", + format!( + "refresh output '{output_id}' must map to env var {expected}, found {resolved}" + ), + )); + } + } + + for required in required_outputs { + if !mapped_required.contains(*required) { + diagnostics.push(ProfileValidationDiagnostic::error( + source, + profile_id, + "credentials.refresh.additional_outputs", + format!( + "strategy {strategy_name} requires additional output '{required}'" + ), + )); + } + } + + if let Some(expected_primary) = strategy_primary_env_key(refresh.strategy) + && (credential.env_vars.len() != 1 + || credential.env_vars[0] != expected_primary) + { + diagnostics.push(ProfileValidationDiagnostic::error( + source, + profile_id, + "credentials.env_vars", + format!( + "strategy {strategy_name} requires primary credential to map to env var {expected_primary}" + ), + )); + } } if let Some(token_grant) = credential.token_grant.as_ref() @@ -2782,4 +3051,340 @@ endpoints: .any(|e| e.host == "s3.dualstack.*.amazonaws.com") ); } + + #[test] + fn aws_profile_declares_additional_outputs() { + for id in ["aws", "aws-s3"] { + let profile = get_default_profile(id).expect("profile should exist"); + let refresh = profile + .credentials + .iter() + .find(|c| c.name == "access_key_id") + .and_then(|c| c.refresh.as_ref()) + .expect("access_key_id refresh should exist"); + let outputs: Vec<(&str, &str)> = refresh + .additional_outputs + .iter() + .map(|o| (o.output.as_str(), o.credential.as_str())) + .collect(); + assert_eq!( + outputs, + vec![ + ("secret_access_key", "secret_access_key"), + ("session_token", "session_token"), + ], + "unexpected additional_outputs for {id}" + ); + } + } + + #[test] + fn aws_profiles_are_runtime_resolvable() { + // With AwsStsAssumeRole recognized as gateway-mintable, all three + // required credentials are runtime-resolvable, so `--runtime-credentials` + // (empty provider creation) is allowed. + for id in ["aws", "aws-s3"] { + let profile = get_default_profile(id).expect("profile should exist"); + assert!( + profile.allows_empty_provider_credentials(), + "{id} should allow empty provider credentials" + ); + } + } + + #[test] + fn is_gateway_mintable_strategy_includes_aws_sts() { + assert!(super::is_gateway_mintable_strategy( + openshell_core::proto::ProviderCredentialRefreshStrategy::AwsStsAssumeRole + )); + } + + #[test] + fn additional_outputs_round_trip_through_proto_and_yaml() { + let profile = parse_profile_yaml( + r" +id: aws-round-trip +display_name: AWS Round Trip +credentials: + - name: access_key_id + env_vars: [AWS_ACCESS_KEY_ID] + required: true + refresh: + strategy: aws_sts_assume_role + additional_outputs: + - output: secret_access_key + credential: secret_access_key + - output: session_token + credential: session_token + material: + - name: role_arn + required: true + - name: secret_access_key + env_vars: [AWS_SECRET_ACCESS_KEY] + required: true + - name: session_token + env_vars: [AWS_SESSION_TOKEN] + required: true +", + ) + .expect("profile should parse"); + + let from_proto = ProviderTypeProfile::from_proto(&profile.to_proto()); + assert_eq!( + from_proto.credentials[0].refresh, + profile.credentials[0].refresh + ); + + let exported = profile_to_yaml(&from_proto).expect("yaml"); + assert!(exported.contains("additional_outputs")); + assert!(exported.contains("session_token")); + let reparsed = parse_profile_yaml(&exported).expect("re-parse"); + assert_eq!( + reparsed.credentials[0].refresh, + profile.credentials[0].refresh + ); + } + + fn aws_output_profile(additional_outputs: &str, credentials_tail: &str) -> ProviderTypeProfile { + parse_profile_yaml(&format!( + r" +id: aws-outputs +display_name: AWS Outputs +credentials: + - name: access_key_id + env_vars: [AWS_ACCESS_KEY_ID] + required: true + refresh: + strategy: aws_sts_assume_role +{additional_outputs} + material: + - name: role_arn + required: true +{credentials_tail} +" + )) + .expect("profile should parse") + } + + const STANDARD_AWS_OUTPUT_TAIL: &str = " - name: secret_access_key + env_vars: [AWS_SECRET_ACCESS_KEY] + required: true + - name: session_token + env_vars: [AWS_SESSION_TOKEN] + required: true"; + + #[test] + fn validate_rejects_additional_output_unknown_credential() { + let profile = aws_output_profile( + " additional_outputs: + - output: secret_access_key + credential: nonexistent + - output: session_token + credential: session_token", + STANDARD_AWS_OUTPUT_TAIL, + ); + let diagnostics = validate_profile_set(&[("aws.yaml".to_string(), profile)]); + assert!( + diagnostics.iter().any(|d| d.field + == "credentials.refresh.additional_outputs.credential" + && d.message.contains("unknown credential 'nonexistent'")), + "diagnostics: {diagnostics:?}" + ); + } + + #[test] + fn validate_rejects_duplicate_additional_output() { + let profile = aws_output_profile( + " additional_outputs: + - output: secret_access_key + credential: secret_access_key + - output: secret_access_key + credential: secret_access_key + - output: session_token + credential: session_token", + STANDARD_AWS_OUTPUT_TAIL, + ); + let diagnostics = validate_profile_set(&[("aws.yaml".to_string(), profile)]); + assert!( + diagnostics + .iter() + .any(|d| d.message.contains("duplicate refresh additional output")), + "diagnostics: {diagnostics:?}" + ); + } + + #[test] + fn validate_rejects_unknown_output_for_strategy() { + let profile = aws_output_profile( + " additional_outputs: + - output: secret_access_key + credential: secret_access_key + - output: session_token + credential: session_token + - output: bogus + credential: session_token", + STANDARD_AWS_OUTPUT_TAIL, + ); + let diagnostics = validate_profile_set(&[("aws.yaml".to_string(), profile)]); + assert!( + diagnostics + .iter() + .any(|d| d.message.contains("does not produce output 'bogus'")), + "diagnostics: {diagnostics:?}" + ); + } + + #[test] + fn validate_rejects_missing_required_output() { + let profile = aws_output_profile( + " additional_outputs: + - output: secret_access_key + credential: secret_access_key", + STANDARD_AWS_OUTPUT_TAIL, + ); + let diagnostics = validate_profile_set(&[("aws.yaml".to_string(), profile)]); + assert!( + diagnostics.iter().any(|d| d + .message + .contains("requires additional output 'session_token'")), + "diagnostics: {diagnostics:?}" + ); + } + + #[test] + fn validate_rejects_output_credential_with_own_refresh() { + let tail = " - name: secret_access_key + env_vars: [AWS_SECRET_ACCESS_KEY] + required: true + refresh: + strategy: oauth2_client_credentials + - name: session_token + env_vars: [AWS_SESSION_TOKEN] + required: true"; + let profile = aws_output_profile( + " additional_outputs: + - output: secret_access_key + credential: secret_access_key + - output: session_token + credential: session_token", + tail, + ); + let diagnostics = validate_profile_set(&[("aws.yaml".to_string(), profile)]); + assert!( + diagnostics + .iter() + .any(|d| d.message.contains("must not declare its own refresh")), + "diagnostics: {diagnostics:?}" + ); + } + + #[test] + fn validate_rejects_nonstandard_aws_output_env_key() { + let tail = " - name: secret_access_key + env_vars: [WRONG_SECRET] + required: true + - name: session_token + env_vars: [AWS_SESSION_TOKEN] + required: true"; + let profile = aws_output_profile( + " additional_outputs: + - output: secret_access_key + credential: secret_access_key + - output: session_token + credential: session_token", + tail, + ); + let diagnostics = validate_profile_set(&[("aws.yaml".to_string(), profile)]); + assert!( + diagnostics.iter().any(|d| d + .message + .contains("must map to env var AWS_SECRET_ACCESS_KEY, found WRONG_SECRET")), + "diagnostics: {diagnostics:?}" + ); + } + + #[test] + fn validate_rejects_wrong_primary_env_key_for_aws_sts() { + let profile = parse_profile_yaml( + r" +id: aws-wrong-primary +display_name: AWS Wrong Primary +credentials: + - name: access_key_id + env_vars: [NOT_AWS_ACCESS_KEY_ID] + required: true + refresh: + strategy: aws_sts_assume_role + additional_outputs: + - output: secret_access_key + credential: secret_access_key + - output: session_token + credential: session_token + material: + - name: role_arn + required: true + - name: secret_access_key + env_vars: [AWS_SECRET_ACCESS_KEY] + required: true + - name: session_token + env_vars: [AWS_SESSION_TOKEN] + required: true +", + ) + .expect("profile should parse"); + let diagnostics = validate_profile_set(&[("aws.yaml".to_string(), profile)]); + assert!( + diagnostics.iter().any(|d| d + .message + .contains("requires primary credential to map to env var AWS_ACCESS_KEY_ID")), + "diagnostics: {diagnostics:?}" + ); + } + + #[test] + fn validate_rejects_additional_outputs_on_unsupported_strategy() { + let profile = parse_profile_yaml( + r" +id: oauth-with-outputs +display_name: OAuth With Outputs +credentials: + - name: access_token + env_vars: [ACCESS_TOKEN] + required: true + refresh: + strategy: oauth2_client_credentials + additional_outputs: + - output: extra + credential: other + - name: other + env_vars: [OTHER] + required: false +", + ) + .expect("profile should parse"); + let diagnostics = validate_profile_set(&[("oauth.yaml".to_string(), profile)]); + assert!( + diagnostics + .iter() + .any(|d| d.message.contains("does not support additional_outputs")), + "diagnostics: {diagnostics:?}" + ); + } + + #[test] + fn validate_accepts_standard_aws_additional_outputs() { + let profile = aws_output_profile( + " additional_outputs: + - output: secret_access_key + credential: secret_access_key + - output: session_token + credential: session_token", + STANDARD_AWS_OUTPUT_TAIL, + ); + let diagnostics = validate_profile_set(&[("aws.yaml".to_string(), profile)]); + assert!( + diagnostics.is_empty(), + "unexpected diagnostics: {diagnostics:?}" + ); + } } diff --git a/crates/openshell-server/src/grpc/provider.rs b/crates/openshell-server/src/grpc/provider.rs index 04a15dcbb4..0a23c14f3e 100644 --- a/crates/openshell-server/src/grpc/provider.rs +++ b/crates/openshell-server/src/grpc/provider.rs @@ -1066,13 +1066,18 @@ async fn active_provider_environment_keys( ) -> Result, Status> { let mut keys = active_provider_credential_keys(provider, now_ms); if !provider.object_id().is_empty() { - keys.extend( + for state in crate::provider_refresh::list_refresh_states_for_provider(store, provider.object_id()) .await? - .into_iter() - .map(|state| state.credential_key) - .filter(|key| is_valid_env_key(key)), - ); + { + // The primary key plus every co-minted output key this refresh owns, + // so a configured-but-not-yet-minted refresh reserves all of them. + keys.extend( + std::iter::once(state.credential_key) + .chain(state.additional_output_keys.into_values()) + .filter(|key| is_valid_env_key(key)), + ); + } } keys.sort(); keys.dedup(); @@ -1512,6 +1517,21 @@ async fn provider_refresh_defaults( .and_then(|credential| credential.refresh.clone())) } +/// Resolve the env keys a refresh co-mints (beyond its primary credential) from +/// the provider's profile `additional_outputs`. Returns semantic output id -> +/// env key. Empty when the provider type has no profile or the credential +/// declares no additional outputs. +async fn resolved_additional_output_keys( + store: &Store, + provider: &Provider, + credential_key: &str, +) -> Result, Status> { + let Some(profile) = get_provider_type_profile(store, &provider.r#type).await? else { + return Ok(std::collections::HashMap::new()); + }; + Ok(profile.resolved_additional_output_keys(credential_key)) +} + fn validate_refresh_material( material: &std::collections::HashMap, refresh_defaults: Option<&CredentialRefreshProfile>, @@ -2076,15 +2096,19 @@ pub(super) async fn handle_configure_provider_refresh( credential_key, ) .await?; - if strategy == ProviderCredentialRefreshStrategy::AwsStsAssumeRole { - for additional_key in ["AWS_SECRET_ACCESS_KEY", "AWS_SESSION_TOKEN"] { - validate_provider_credential_key_available_for_attached_sandboxes( - state.store.as_ref(), - &provider, - additional_key, - ) - .await?; - } + // Reserve every env key this refresh will co-mint (resolved from the + // profile's additional_outputs), not just the primary, so a + // configured-but-not-yet-minted refresh can't collide with another provider + // that later claims one of the derived keys. + let additional_output_keys = + resolved_additional_output_keys(state.store.as_ref(), &provider, credential_key).await?; + for additional_key in additional_output_keys.values() { + validate_provider_credential_key_available_for_attached_sandboxes( + state.store.as_ref(), + &provider, + additional_key, + ) + .await?; } let refresh_defaults = provider_refresh_defaults(state.store.as_ref(), &provider, credential_key).await?; @@ -2152,6 +2176,7 @@ pub(super) async fn handle_configure_provider_refresh( scopes, refresh_before_seconds, max_lifetime_seconds, + additional_output_keys, }, )?; if let Some(existing) = existing_refresh_state { @@ -3061,6 +3086,7 @@ mod tests { scopes: Vec::new(), refresh_before_seconds: 300, max_lifetime_seconds: 3600, + additional_outputs: Vec::new(), material: vec![ ProviderCredentialRefreshMaterial { name: "client_id".to_string(), @@ -4390,6 +4416,7 @@ mod tests { &provider, "API_TOKEN", crate::provider_refresh::NewRefreshStateConfig { + additional_output_keys: HashMap::new(), strategy: ProviderCredentialRefreshStrategy::External, material: HashMap::from([( "endpoint".to_string(), @@ -4599,6 +4626,7 @@ mod tests { scopes: vec!["https://example.test/.default".to_string()], refresh_before_seconds: 300, max_lifetime_seconds: 3600, + additional_outputs: Vec::new(), material: vec![ ProviderCredentialRefreshMaterial { name: "client_id".to_string(), @@ -6171,6 +6199,151 @@ mod tests { ); } + #[tokio::test] + async fn configure_aws_sts_persists_resolved_additional_output_keys() { + use crate::grpc::StoredSettingValue; + use crate::grpc::StoredSettings; + use crate::grpc::policy::save_global_settings; + + let state = test_server_state().await; + let global_settings = StoredSettings { + revision: 1, + settings: std::iter::once(( + openshell_core::settings::PROVIDERS_V2_ENABLED_KEY.to_string(), + StoredSettingValue::Bool(true), + )) + .collect(), + ..Default::default() + }; + save_global_settings(state.store.as_ref(), &global_settings) + .await + .unwrap(); + + create_provider_record( + state.store.as_ref(), + Provider { + metadata: Some(openshell_core::proto::datamodel::v1::ObjectMeta { + id: String::new(), + name: "aws-outputs".to_string(), + created_at_ms: 0, + labels: HashMap::new(), + resource_version: 0, + }), + r#type: "aws".to_string(), + credentials: HashMap::new(), + config: HashMap::new(), + credential_expires_at_ms: HashMap::new(), + }, + ) + .await + .unwrap(); + + handle_configure_provider_refresh( + &state, + Request::new(ConfigureProviderRefreshRequest { + provider: "aws-outputs".to_string(), + credential_key: "AWS_ACCESS_KEY_ID".to_string(), + strategy: ProviderCredentialRefreshStrategy::AwsStsAssumeRole as i32, + material: HashMap::from([( + "role_arn".to_string(), + "arn:aws:iam::123456789012:role/Test".to_string(), + )]), + secret_material_keys: Vec::new(), + expires_at_ms: None, + }), + ) + .await + .unwrap(); + + let provider = state + .store + .get_message_by_name::("aws-outputs") + .await + .unwrap() + .unwrap(); + let stored = crate::provider_refresh::get_refresh_state( + state.store.as_ref(), + provider.object_id(), + "AWS_ACCESS_KEY_ID", + ) + .await + .unwrap() + .expect("refresh state should exist"); + + assert_eq!( + stored.additional_output_keys.get("secret_access_key"), + Some(&"AWS_SECRET_ACCESS_KEY".to_string()) + ); + assert_eq!( + stored.additional_output_keys.get("session_token"), + Some(&"AWS_SESSION_TOKEN".to_string()) + ); + } + + #[tokio::test] + async fn active_provider_environment_keys_include_additional_output_keys() { + let state = test_server_state().await; + create_provider_record( + state.store.as_ref(), + Provider { + metadata: Some(openshell_core::proto::datamodel::v1::ObjectMeta { + id: String::new(), + name: "aws-env".to_string(), + created_at_ms: 0, + labels: HashMap::new(), + resource_version: 0, + }), + r#type: "aws".to_string(), + credentials: HashMap::new(), + config: HashMap::new(), + credential_expires_at_ms: HashMap::new(), + }, + ) + .await + .unwrap(); + let provider = state + .store + .get_message_by_name::("aws-env") + .await + .unwrap() + .unwrap(); + + // A configured-but-not-yet-minted STS refresh: no minted values are in + // provider.credentials yet, but the reserved keys must still surface. + let refresh_state = crate::provider_refresh::new_refresh_state( + &provider, + "AWS_ACCESS_KEY_ID", + crate::provider_refresh::NewRefreshStateConfig { + additional_output_keys: HashMap::from([ + ( + "secret_access_key".to_string(), + "AWS_SECRET_ACCESS_KEY".to_string(), + ), + ("session_token".to_string(), "AWS_SESSION_TOKEN".to_string()), + ]), + strategy: ProviderCredentialRefreshStrategy::AwsStsAssumeRole, + material: HashMap::new(), + secret_material_keys: Vec::new(), + expires_at_ms: 0, + token_url: String::new(), + scopes: Vec::new(), + refresh_before_seconds: 0, + max_lifetime_seconds: 0, + }, + ) + .unwrap(); + crate::provider_refresh::put_refresh_state(state.store.as_ref(), &refresh_state) + .await + .unwrap(); + + let keys = active_provider_environment_keys(state.store.as_ref(), &provider, 0) + .await + .unwrap(); + assert!(keys.contains(&"AWS_ACCESS_KEY_ID".to_string())); + assert!(keys.contains(&"AWS_SECRET_ACCESS_KEY".to_string())); + assert!(keys.contains(&"AWS_SESSION_TOKEN".to_string())); + } + #[tokio::test] async fn configure_aws_sts_validates_additional_credential_key_collision() { use crate::grpc::StoredSettingValue; diff --git a/crates/openshell-server/src/provider_refresh.rs b/crates/openshell-server/src/provider_refresh.rs index 9ed9f1f899..ea58dffe89 100644 --- a/crates/openshell-server/src/provider_refresh.rs +++ b/crates/openshell-server/src/provider_refresh.rs @@ -176,6 +176,10 @@ pub struct NewRefreshStateConfig { pub scopes: Vec, pub refresh_before_seconds: i64, pub max_lifetime_seconds: i64, + /// Resolved semantic output id -> concrete env key for credentials this + /// refresh co-mints beyond its primary. Pinned from the profile's + /// `additional_outputs` at configure time. + pub additional_output_keys: HashMap, } #[allow(clippy::unnecessary_wraps)] @@ -216,6 +220,7 @@ pub fn new_refresh_state( scopes: config.scopes, refresh_before_seconds: config.refresh_before_seconds, max_lifetime_seconds: config.max_lifetime_seconds, + additional_output_keys: config.additional_output_keys, }) } @@ -285,15 +290,7 @@ pub fn refresh_strategy_name(strategy: i32) -> &'static str { } } -pub fn is_gateway_mintable_strategy(strategy: ProviderCredentialRefreshStrategy) -> bool { - matches!( - strategy, - ProviderCredentialRefreshStrategy::Oauth2RefreshToken - | ProviderCredentialRefreshStrategy::Oauth2ClientCredentials - | ProviderCredentialRefreshStrategy::GoogleServiceAccountJwt - | ProviderCredentialRefreshStrategy::AwsStsAssumeRole - ) -} +pub use openshell_providers::is_gateway_mintable_strategy; pub async fn refresh_provider_credential( store: &Store, @@ -651,9 +648,27 @@ async fn mint_aws_sts_assume_role( let max_expires = now_ms + max_lifetime_i64 * 1000; let expires_at_ms = expires_at_ms.min(max_expires); + // Map STS response fields to the env keys the profile bound to each + // semantic output. Fall back to the standard AWS names when the state has + // no resolved mapping (older refresh states configured before + // additional_outputs existed). Profile validation guarantees these resolve + // to the standard names the SigV4 signer expects. + let output_values = [ + ( + "secret_access_key", + secret_access_key, + "AWS_SECRET_ACCESS_KEY", + ), + ("session_token", session_token, "AWS_SESSION_TOKEN"), + ]; let mut additional = HashMap::new(); - additional.insert("AWS_SECRET_ACCESS_KEY".to_string(), secret_access_key); - additional.insert("AWS_SESSION_TOKEN".to_string(), session_token); + for (output_id, value, default_key) in output_values { + let env_key = state + .additional_output_keys + .get(output_id) + .map_or(default_key, String::as_str); + additional.insert(env_key.to_string(), value); + } Ok(MintedCredential { access_token: access_key_id, @@ -974,6 +989,7 @@ mod tests { &provider, "MS_GRAPH_ACCESS_TOKEN", NewRefreshStateConfig { + additional_output_keys: HashMap::new(), strategy: ProviderCredentialRefreshStrategy::Oauth2ClientCredentials, material: HashMap::from([ ("client_id".to_string(), "client-id".to_string()), @@ -1057,6 +1073,7 @@ mod tests { &provider_b, "MS_GRAPH_ACCESS_TOKEN", NewRefreshStateConfig { + additional_output_keys: HashMap::new(), strategy: ProviderCredentialRefreshStrategy::Oauth2ClientCredentials, material: HashMap::from([ ("client_id".to_string(), "client-id".to_string()), @@ -1125,6 +1142,7 @@ mod tests { &provider, "MS_GRAPH_ACCESS_TOKEN", NewRefreshStateConfig { + additional_output_keys: HashMap::new(), strategy: ProviderCredentialRefreshStrategy::Oauth2RefreshToken, material: HashMap::from([ ("client_id".to_string(), "client-id".to_string()), @@ -1204,6 +1222,7 @@ mod tests { &provider, "GOOGLE_DRIVE_ACCESS_TOKEN", NewRefreshStateConfig { + additional_output_keys: HashMap::new(), strategy: ProviderCredentialRefreshStrategy::GoogleServiceAccountJwt, material: HashMap::from([ ( @@ -1250,6 +1269,7 @@ mod tests { &provider, "MS_GRAPH_ACCESS_TOKEN", NewRefreshStateConfig { + additional_output_keys: HashMap::new(), strategy: ProviderCredentialRefreshStrategy::External, material: HashMap::new(), secret_material_keys: Vec::new(), @@ -1328,6 +1348,7 @@ mod tests { &prov, "AWS_ACCESS_KEY_ID", NewRefreshStateConfig { + additional_output_keys: HashMap::new(), strategy: ProviderCredentialRefreshStrategy::AwsStsAssumeRole, material: HashMap::from([ ( @@ -1378,6 +1399,88 @@ mod tests { ); } + #[tokio::test] + async fn aws_sts_mint_writes_to_resolved_additional_output_keys() { + let mock_server = MockServer::start().await; + Mock::given(method("POST")) + .and(body_string_contains("Action=AssumeRole")) + .respond_with(ResponseTemplate::new(200).set_body_string( + r#" + + + ASIAMOCKKEY + MockSecretAccessKey123 + MockSessionTokenXYZ + 2099-01-01T00:00:00Z + + +"#, + )) + .mount(&mock_server) + .await; + + let store = test_store().await; + let prov = provider("aws-sts-custom", "aws"); + store.put_message(&prov).await.unwrap(); + + // The minter honors the resolved output->env-key map from state, not + // hardcoded AWS names. + let state = new_refresh_state( + &prov, + "AWS_ACCESS_KEY_ID", + NewRefreshStateConfig { + additional_output_keys: HashMap::from([ + ("secret_access_key".to_string(), "CUSTOM_SECRET".to_string()), + ("session_token".to_string(), "CUSTOM_SESSION".to_string()), + ]), + strategy: ProviderCredentialRefreshStrategy::AwsStsAssumeRole, + material: HashMap::from([ + ( + "role_arn".to_string(), + "arn:aws:iam::123456789012:role/TestRole".to_string(), + ), + ("aws_access_key_id".to_string(), "AKIATESTKEY".to_string()), + ( + "aws_secret_access_key".to_string(), + "TestSecretKey".to_string(), + ), + ("sts_endpoint_url".to_string(), mock_server.uri()), + ]), + secret_material_keys: vec!["aws_secret_access_key".to_string()], + expires_at_ms: 0, + token_url: String::new(), + scopes: Vec::new(), + refresh_before_seconds: 300, + max_lifetime_seconds: 3600, + }, + ) + .unwrap(); + put_refresh_state(&store, &state).await.unwrap(); + + refresh_provider_credential(&store, "aws-sts-custom", "AWS_ACCESS_KEY_ID") + .await + .unwrap(); + + let stored = store + .get_message_by_name::("aws-sts-custom") + .await + .unwrap() + .unwrap(); + assert_eq!( + stored.credentials.get("AWS_ACCESS_KEY_ID"), + Some(&"ASIAMOCKKEY".to_string()) + ); + assert_eq!( + stored.credentials.get("CUSTOM_SECRET"), + Some(&"MockSecretAccessKey123".to_string()) + ); + assert_eq!( + stored.credentials.get("CUSTOM_SESSION"), + Some(&"MockSessionTokenXYZ".to_string()) + ); + assert!(!stored.credentials.contains_key("AWS_SECRET_ACCESS_KEY")); + } + #[tokio::test] async fn apply_minted_credential_writes_additional_keys() { use super::apply_minted_credential; diff --git a/docs/providers/aws-sigv4.mdx b/docs/providers/aws-sigv4.mdx index fc5e02040b..61465fa1b8 100644 --- a/docs/providers/aws-sigv4.mdx +++ b/docs/providers/aws-sigv4.mdx @@ -35,6 +35,12 @@ openshell provider create \ --credential AWS_SESSION_TOKEN=FwoGZX... ``` +To have the gateway mint and rotate STS credentials for you instead of supplying +them statically, use the `aws` or `aws-s3` profile with the `aws_sts_assume_role` +refresh strategy. A single `sts:AssumeRole` mints all three env vars +(`AWS_ACCESS_KEY_ID`, `AWS_SECRET_ACCESS_KEY`, `AWS_SESSION_TOKEN`) that the +signer reads. See [Manage Providers](/sandboxes/manage-providers#aws-sts). + ## Policy Configuration Enable SigV4 signing on a per-endpoint basis using three policy fields: diff --git a/docs/sandboxes/manage-providers.mdx b/docs/sandboxes/manage-providers.mdx index 98e706be6e..218fa697b0 100644 --- a/docs/sandboxes/manage-providers.mdx +++ b/docs/sandboxes/manage-providers.mdx @@ -182,11 +182,15 @@ openshell provider refresh rotate my-graph --credential-key MS_GRAPH_ACCESS_TOKE AWS STS refresh requires `providers_v2_enabled=true`. The gateway calls `sts:AssumeRole` and writes three short-lived credentials (`AWS_ACCESS_KEY_ID`, `AWS_SECRET_ACCESS_KEY`, `AWS_SESSION_TOKEN`) to the provider record atomically. +The `aws` and `aws-s3` profiles declare `AWS_SECRET_ACCESS_KEY` and +`AWS_SESSION_TOKEN` as `additional_outputs` of the `AWS_ACCESS_KEY_ID` refresh, +so all three credentials are gateway-minted. Create the provider with +`--runtime-credentials` — no placeholder credential is needed. ```shell openshell settings set --global --key providers_v2_enabled --value true --yes -openshell provider create --name my-aws --type aws-s3 +openshell provider create --name my-aws --type aws-s3 --runtime-credentials openshell provider refresh configure my-aws \ --credential-key AWS_ACCESS_KEY_ID \ diff --git a/docs/sandboxes/providers-v2.mdx b/docs/sandboxes/providers-v2.mdx index 8ccb7859e9..13836bc8c6 100644 --- a/docs/sandboxes/providers-v2.mdx +++ b/docs/sandboxes/providers-v2.mdx @@ -300,8 +300,9 @@ Profile YAML can declare these refresh strategies: | `oauth2_refresh_token` | The gateway exchanges a refresh token for a short-lived access token. | | `oauth2_client_credentials` | The gateway mints a short-lived access token with OAuth2 client credentials. | | `google_service_account_jwt` | The gateway signs a Google service account JWT and exchanges it for an access token. | +| `aws_sts_assume_role` | The gateway calls `sts:AssumeRole` and mints `AWS_ACCESS_KEY_ID`, `AWS_SECRET_ACCESS_KEY`, and `AWS_SESSION_TOKEN` in one operation. Requires `providers_v2_enabled=true`. | -`openshell provider refresh configure` accepts only gateway-mintable strategies: `oauth2-refresh-token`, `oauth2-client-credentials`, and `google-service-account-jwt`. Use `openshell provider update` for `static` and `external` refresh patterns. +`openshell provider refresh configure` accepts only gateway-mintable strategies: `oauth2-refresh-token`, `oauth2-client-credentials`, `google-service-account-jwt`, and `aws-sts-assume-role`. Use `openshell provider update` for `static` and `external` refresh patterns. Gateway-managed refresh strategies use these material keys: @@ -310,9 +311,36 @@ Gateway-managed refresh strategies use these material keys: | `oauth2_refresh_token` | `client_id`, `refresh_token`, optional `client_secret`. | | `oauth2_client_credentials` | `client_id`, `client_secret`, optional `tenant_id` for Microsoft Entra token URLs. | | `google_service_account_jwt` | `client_email`, `private_key`, optional `subject` or `sub`. | +| `aws_sts_assume_role` | `role_arn`, optional `session_name`, `external_id`, `aws_region`, and optional long-lived `aws_access_key_id` / `aws_secret_access_key` for gateways without ambient AWS credentials. | OpenShell keeps token endpoints profile-owned. Refresh material cannot override `token_url` or `token_uri` during refresh configuration. +### Additional Outputs + +Most refresh strategies mint a single credential. `aws_sts_assume_role` mints three. A refresh declares the extra credentials it co-mints with `additional_outputs`, mapping each strategy-defined output id to a sibling credential whose `env_vars` receive the value: + +```yaml +credentials: + - name: access_key_id + env_vars: [AWS_ACCESS_KEY_ID] + required: true + refresh: + strategy: aws_sts_assume_role + additional_outputs: + - output: secret_access_key + credential: secret_access_key + - output: session_token + credential: session_token + - name: secret_access_key + env_vars: [AWS_SECRET_ACCESS_KEY] + required: true + - name: session_token + env_vars: [AWS_SESSION_TOKEN] + required: true +``` + +The refresh attaches to the primary credential (`access_key_id`). Each referenced sibling must exist, declare exactly one env var, and not carry its own refresh. Because a refresh's outputs are runtime-resolvable, a profile whose only credentials are STS-minted can be created with `--runtime-credentials`. The resolved output-to-env-key mapping is pinned when refresh is configured, so later profile edits do not silently redirect where minted values are written. + ### Dynamic Token Grants `token_grant` belongs to one credential declaration. When a sandbox with the provider attached sends HTTP traffic to a matching profile endpoint, the supervisor requests a SPIFFE JWT-SVID from the local Workload API, exchanges it at `token_endpoint`, caches the returned access token, and injects it before forwarding the request upstream. Use `auth_style: bearer` to inject `Authorization: Bearer `, or `auth_style: header` with `header_name` to inject the raw access token into a custom header. Token grants do not support `query` or `path` placement. diff --git a/examples/aws-s3-sts.md b/examples/aws-s3-sts.md index 26ae035942..7e41995077 100644 --- a/examples/aws-s3-sts.md +++ b/examples/aws-s3-sts.md @@ -132,9 +132,9 @@ export OPENSHELL_BASE_URL=http://localhost:18080 # Enable provider v2 (required for STS) openshell settings set --global --key providers_v2_enabled --value true --yes -# Create the provider with the aws-s3 profile -openshell provider create --name s3-test --type aws-s3 \ - --credential AWS_ACCESS_KEY_ID=placeholder +# Create the provider with the aws-s3 profile. All three credentials are +# gateway-minted via STS, so no static credential is needed. +openshell provider create --name s3-test --type aws-s3 --runtime-credentials # Configure STS refresh openshell provider refresh configure s3-test \ diff --git a/proto/openshell.proto b/proto/openshell.proto index bf49d2b391..5a57401d9b 100644 --- a/proto/openshell.proto +++ b/proto/openshell.proto @@ -984,6 +984,15 @@ message ProviderCredentialRefreshMaterial { bool secret = 4; } +// Declares that a single refresh operation mints more than one credential. +// The refresh is attached to a primary credential; each additional output +// maps a strategy-defined semantic output id to a sibling credential whose +// env_vars receive the minted value. +message ProviderCredentialRefreshOutput { + string output = 1; // strategy-defined semantic output id (e.g. "session_token") + string credential = 2; // sibling credential name whose env_vars receive this output +} + message ProviderCredentialRefresh { ProviderCredentialRefreshStrategy strategy = 1; string token_url = 2; @@ -991,6 +1000,7 @@ message ProviderCredentialRefresh { int64 refresh_before_seconds = 4; int64 max_lifetime_seconds = 5; repeated ProviderCredentialRefreshMaterial material = 6; + repeated ProviderCredentialRefreshOutput additional_outputs = 7; } message ProviderCredentialRefreshStatus { @@ -1028,6 +1038,11 @@ message StoredProviderCredentialRefreshState { repeated string scopes = 14; int64 refresh_before_seconds = 15; int64 max_lifetime_seconds = 16; + // Resolved mapping of strategy-defined output id -> concrete env key, pinned + // at configure time from the profile's additional_outputs. Read by minting, + // collision reservation, and env-key surfacing so later profile edits cannot + // silently redirect writes. + map additional_output_keys = 17; } message GetProviderRefreshStatusRequest { diff --git a/providers/aws-s3.yaml b/providers/aws-s3.yaml index 87d5350e14..e8885924ee 100644 --- a/providers/aws-s3.yaml +++ b/providers/aws-s3.yaml @@ -14,6 +14,14 @@ credentials: strategy: aws_sts_assume_role refresh_before_seconds: 300 max_lifetime_seconds: 3600 + # One AssumeRole call mints all three AWS credentials. The refresh is + # attached to access_key_id (primary); these outputs co-manage the + # secret key and session token on their sibling credentials. + additional_outputs: + - output: secret_access_key + credential: secret_access_key + - output: session_token + credential: session_token material: - name: role_arn description: ARN of the IAM role to assume diff --git a/providers/aws.yaml b/providers/aws.yaml index 41323d204d..d63795ff58 100644 --- a/providers/aws.yaml +++ b/providers/aws.yaml @@ -14,6 +14,14 @@ credentials: strategy: aws_sts_assume_role refresh_before_seconds: 300 max_lifetime_seconds: 3600 + # One AssumeRole call mints all three AWS credentials. The refresh is + # attached to access_key_id (primary); these outputs co-manage the + # secret key and session token on their sibling credentials. + additional_outputs: + - output: secret_access_key + credential: secret_access_key + - output: session_token + credential: session_token material: - name: role_arn description: ARN of the IAM role to assume From e7a978c8224aad8053642b664188f64fe5a22192 Mon Sep 17 00:00:00 2001 From: Russell Bryant Date: Tue, 14 Jul 2026 14:57:44 -0400 Subject: [PATCH 03/16] fix(providers): reject caller-supplied STS endpoint override The aws_sts_assume_role refresh read an sts_endpoint_url from refresh material and, when it pointed at a loopback host, redirected the AWS-signed AssumeRole request there. That let a caller aim a signed STS request at an arbitrary loopback service (CWE-918 SSRF) via undocumented production material. Compile the endpoint override out of production builds: it is now a test-only helper that unit tests use to target a local mock STS. Reject the sts_endpoint_url material key at the configure boundary so it can never reach a stored refresh state outside tests. Production always resolves the STS endpoint from the region. Signed-off-by: Russell Bryant --- crates/openshell-server/src/grpc/provider.rs | 93 +++++++++++++++++++ .../openshell-server/src/provider_refresh.rs | 30 +++++- 2 files changed, 119 insertions(+), 4 deletions(-) diff --git a/crates/openshell-server/src/grpc/provider.rs b/crates/openshell-server/src/grpc/provider.rs index 0a23c14f3e..84f098012d 100644 --- a/crates/openshell-server/src/grpc/provider.rs +++ b/crates/openshell-server/src/grpc/provider.rs @@ -2075,6 +2075,15 @@ pub(super) async fn handle_configure_provider_refresh( "refresh token endpoints must be defined by the provider profile, not material", )); } + // The STS endpoint is always resolved from the region in production. Reject + // any caller-supplied override so an AWS-signed AssumeRole request cannot be + // redirected at an arbitrary service (CWE-918). Tests inject a mock endpoint + // through test-only code paths, not this boundary. + if request.material.contains_key("sts_endpoint_url") { + return Err(Status::invalid_argument( + "sts_endpoint_url material is not permitted", + )); + } if request .expires_at_ms .is_some_and(|expires_at_ms| expires_at_ms < 0) @@ -6199,6 +6208,90 @@ mod tests { ); } + #[tokio::test] + async fn configure_aws_sts_rejects_endpoint_override_material() { + use crate::grpc::StoredSettingValue; + use crate::grpc::StoredSettings; + use crate::grpc::policy::save_global_settings; + + let state = test_server_state().await; + let global_settings = StoredSettings { + revision: 1, + settings: std::iter::once(( + openshell_core::settings::PROVIDERS_V2_ENABLED_KEY.to_string(), + StoredSettingValue::Bool(true), + )) + .collect(), + ..Default::default() + }; + save_global_settings(state.store.as_ref(), &global_settings) + .await + .unwrap(); + + create_provider_record( + state.store.as_ref(), + Provider { + metadata: Some(openshell_core::proto::datamodel::v1::ObjectMeta { + id: String::new(), + name: "aws-endpoint-override".to_string(), + created_at_ms: 0, + labels: HashMap::new(), + resource_version: 0, + }), + r#type: "aws".to_string(), + credentials: HashMap::new(), + config: HashMap::new(), + credential_expires_at_ms: HashMap::new(), + }, + ) + .await + .unwrap(); + + let err = handle_configure_provider_refresh( + &state, + Request::new(ConfigureProviderRefreshRequest { + provider: "aws-endpoint-override".to_string(), + credential_key: "AWS_ACCESS_KEY_ID".to_string(), + strategy: ProviderCredentialRefreshStrategy::AwsStsAssumeRole as i32, + material: HashMap::from([ + ( + "role_arn".to_string(), + "arn:aws:iam::123456789012:role/Test".to_string(), + ), + ( + "sts_endpoint_url".to_string(), + "http://127.0.0.1:1/".to_string(), + ), + ]), + secret_material_keys: Vec::new(), + expires_at_ms: None, + }), + ) + .await + .unwrap_err(); + + assert_eq!(err.code(), Code::InvalidArgument); + assert!(err.message().contains("sts_endpoint_url")); + + // The rejected configuration must not leave a refresh state behind. + let provider = state + .store + .get_message_by_name::("aws-endpoint-override") + .await + .unwrap() + .unwrap(); + assert!( + crate::provider_refresh::get_refresh_state( + state.store.as_ref(), + provider.object_id(), + "AWS_ACCESS_KEY_ID", + ) + .await + .unwrap() + .is_none() + ); + } + #[tokio::test] async fn configure_aws_sts_persists_resolved_additional_output_keys() { use crate::grpc::StoredSettingValue; diff --git a/crates/openshell-server/src/provider_refresh.rs b/crates/openshell-server/src/provider_refresh.rs index ea58dffe89..7cf10bb893 100644 --- a/crates/openshell-server/src/provider_refresh.rs +++ b/crates/openshell-server/src/provider_refresh.rs @@ -600,10 +600,11 @@ async fn mint_aws_sts_assume_role( let sdk_config = config_loader.load().await; let sts_config = { let mut builder = aws_sdk_sts::config::Builder::from(&sdk_config); - if let Some(endpoint) = material_value(&state.material, &["sts_endpoint_url"]) - && let Ok(parsed) = reqwest::Url::parse(&endpoint) - && parsed.host_str().is_some_and(is_loopback_host) - { + // Endpoint overrides exist only to point tests at a local mock STS. In + // production the endpoint is always resolved from the region so a caller + // cannot redirect an AWS-signed AssumeRole request at an arbitrary + // service (CWE-918). See `test_sts_endpoint_override`. + if let Some(endpoint) = test_sts_endpoint_override(state) { builder = builder.endpoint_url(endpoint); } builder.build() @@ -815,6 +816,27 @@ fn is_loopback_host(host: &str) -> bool { matches!(host, "localhost" | "127.0.0.1" | "::1") } +/// Test-only STS endpoint override. Reads the `sts_endpoint_url` material and +/// accepts it only when it is a loopback URL, so unit tests can target a local +/// mock STS. Compiled out of production builds entirely: the configure boundary +/// also rejects the `sts_endpoint_url` material key, so it can never reach a +/// stored refresh state outside tests. +#[cfg(test)] +fn test_sts_endpoint_override(state: &StoredProviderCredentialRefreshState) -> Option { + material_value(&state.material, &["sts_endpoint_url"]).filter(|endpoint| { + reqwest::Url::parse(endpoint) + .ok() + .and_then(|parsed| parsed.host_str().map(is_loopback_host)) + .unwrap_or(false) + }) +} + +#[cfg(not(test))] +#[allow(clippy::missing_const_for_fn)] +fn test_sts_endpoint_override(_state: &StoredProviderCredentialRefreshState) -> Option { + None +} + pub fn spawn_refresh_worker(state: std::sync::Arc, interval: Duration) { info!( interval_seconds = interval.as_secs(), From 916e9d2bf51329db936ca8e45b2e4769d9f8333b Mon Sep 17 00:00:00 2001 From: Russell Bryant Date: Tue, 14 Jul 2026 15:01:55 -0400 Subject: [PATCH 04/16] fix(providers): validate canonical AWS STS profile binding at configure Configuring an aws_sts_assume_role refresh accepted any provider and credential key, even a generic provider with no profile or a co-managed output key. The mint path then fell back to hardcoded AWS_SECRET_ACCESS_KEY and AWS_SESSION_TOKEN names, so a refresh with no resolved output mapping would still write credentials the SigV4 signer trusts by name (CWE-20). Require a single canonical profile binding at the configure boundary: the credential's profile must declare a matching refresh strategy, the credential key must be the canonical primary env key (AWS_ACCESS_KEY_ID), and every required strategy output must resolve to its canonical env key from the profile's additional_outputs. The canonical primary key is stored as the refresh credential key. Remove the mint-path fallback so a missing resolved output mapping fails closed instead of guessing. Signed-off-by: Russell Bryant --- crates/openshell-providers/src/lib.rs | 3 +- crates/openshell-providers/src/profiles.rs | 13 +- crates/openshell-server/src/grpc/provider.rs | 185 +++++++++++++++++- .../openshell-server/src/provider_refresh.rs | 38 ++-- 4 files changed, 214 insertions(+), 25 deletions(-) diff --git a/crates/openshell-providers/src/lib.rs b/crates/openshell-providers/src/lib.rs index 45408ea31e..a689201a55 100644 --- a/crates/openshell-providers/src/lib.rs +++ b/crates/openshell-providers/src/lib.rs @@ -21,7 +21,8 @@ pub use profiles::{ CredentialRefreshProfile, ProfileError, ProfileValidationDiagnostic, ProviderTypeProfile, default_profiles, get_default_profile, is_gateway_mintable_strategy, normalize_profile_id, parse_profile_json, parse_profile_yaml, profile_to_json, profile_to_yaml, profiles_to_json, - profiles_to_yaml, validate_profile_set, + profiles_to_yaml, strategy_output_env_key, strategy_output_spec, strategy_primary_env_key, + validate_profile_set, }; #[derive(Debug, thiserror::Error)] diff --git a/crates/openshell-providers/src/profiles.rs b/crates/openshell-providers/src/profiles.rs index ee8b2be4bf..df4154cdc2 100644 --- a/crates/openshell-providers/src/profiles.rs +++ b/crates/openshell-providers/src/profiles.rs @@ -562,9 +562,10 @@ pub fn is_gateway_mintable_strategy(strategy: ProviderCredentialRefreshStrategy) } /// Semantic output ids a refresh strategy can produce **in addition** to its -/// primary credential, split into required and optional. Drives -/// `additional_outputs` validation. Strategies not listed here produce only a -/// single (primary) credential and reject any `additional_outputs`. +/// primary credential, split into required and optional. +/// +/// Drives `additional_outputs` validation. Strategies not listed here produce +/// only a single (primary) credential and reject any `additional_outputs`. #[must_use] pub fn strategy_output_spec( strategy: ProviderCredentialRefreshStrategy, @@ -578,8 +579,10 @@ pub fn strategy_output_spec( } /// Expected primary env key for strategies that constrain it, or `None` when the -/// strategy accepts any caller-chosen primary key. AWS `SigV4` signing looks up -/// `AWS_ACCESS_KEY_ID` by name, so STS pins the primary key. +/// strategy accepts any caller-chosen primary key. +/// +/// AWS `SigV4` signing looks up `AWS_ACCESS_KEY_ID` by name, so STS pins the +/// primary key. #[must_use] pub fn strategy_primary_env_key( strategy: ProviderCredentialRefreshStrategy, diff --git a/crates/openshell-server/src/grpc/provider.rs b/crates/openshell-server/src/grpc/provider.rs index 84f098012d..b0616005bf 100644 --- a/crates/openshell-server/src/grpc/provider.rs +++ b/crates/openshell-server/src/grpc/provider.rs @@ -1147,7 +1147,8 @@ use openshell_core::proto::{ }; use openshell_providers::{ CredentialRefreshProfile, ProfileValidationDiagnostic, ProviderTypeProfile, default_profiles, - get_default_profile, normalize_profile_id, normalize_provider_type, validate_profile_set, + get_default_profile, normalize_profile_id, normalize_provider_type, strategy_output_env_key, + strategy_output_spec, strategy_primary_env_key, validate_profile_set, }; use std::sync::Arc; use tonic::{Request, Response}; @@ -1532,6 +1533,54 @@ async fn resolved_additional_output_keys( Ok(profile.resolved_additional_output_keys(credential_key)) } +/// Validate that a strategy which pins canonical env keys (today only +/// `aws_sts_assume_role`) is configured against a matching profile binding. +/// +/// For such strategies the `SigV4` signer resolves credentials by fixed env +/// name, so a caller must not be able to configure the refresh on an arbitrary +/// credential key or on a provider whose profile does not declare it. This +/// requires: (1) a profile refresh declaration for `credential_key` whose +/// strategy matches, (2) `credential_key` equal to the canonical primary env +/// key, and (3) every required strategy output resolved to its canonical env key +/// via the profile's `additional_outputs`. Unconstrained strategies pass through. +fn validate_strategy_profile_binding( + strategy: ProviderCredentialRefreshStrategy, + credential_key: &str, + refresh_defaults: Option<&CredentialRefreshProfile>, + additional_output_keys: &std::collections::HashMap, +) -> Result<(), Status> { + let Some(expected_primary) = strategy_primary_env_key(strategy) else { + return Ok(()); + }; + let strategy_name = crate::provider_refresh::refresh_strategy_name(strategy as i32); + let refresh_defaults = refresh_defaults.ok_or_else(|| { + Status::failed_precondition(format!( + "strategy '{strategy_name}' requires a provider profile that declares this refresh on credential {expected_primary}" + )) + })?; + if refresh_defaults.strategy != strategy { + return Err(Status::failed_precondition(format!( + "provider profile declares strategy '{}' for {credential_key}, not '{strategy_name}'", + crate::provider_refresh::refresh_strategy_name(refresh_defaults.strategy as i32) + ))); + } + if credential_key != expected_primary { + return Err(Status::failed_precondition(format!( + "strategy '{strategy_name}' requires credential_key {expected_primary}, got {credential_key}" + ))); + } + let (required_outputs, _optional_outputs) = strategy_output_spec(strategy); + for output in required_outputs { + let expected_env = strategy_output_env_key(strategy, output).unwrap_or_default(); + if additional_output_keys.get(*output).map(String::as_str) != Some(expected_env) { + return Err(Status::failed_precondition(format!( + "strategy '{strategy_name}' requires profile additional output '{output}' bound to {expected_env}" + ))); + } + } + Ok(()) +} + fn validate_refresh_material( material: &std::collections::HashMap, refresh_defaults: Option<&CredentialRefreshProfile>, @@ -2121,6 +2170,12 @@ pub(super) async fn handle_configure_provider_refresh( } let refresh_defaults = provider_refresh_defaults(state.store.as_ref(), &provider, credential_key).await?; + validate_strategy_profile_binding( + strategy, + credential_key, + refresh_defaults.as_ref(), + &additional_output_keys, + )?; validate_refresh_material(&request.material, refresh_defaults.as_ref())?; let material_scopes = crate::provider_refresh::material_scopes(&request.material); let token_url = refresh_defaults @@ -6373,6 +6428,134 @@ mod tests { ); } + #[tokio::test] + async fn configure_aws_sts_requires_profile_declaring_the_refresh() { + use crate::grpc::StoredSettingValue; + use crate::grpc::StoredSettings; + use crate::grpc::policy::save_global_settings; + + let state = test_server_state().await; + let global_settings = StoredSettings { + revision: 1, + settings: std::iter::once(( + openshell_core::settings::PROVIDERS_V2_ENABLED_KEY.to_string(), + StoredSettingValue::Bool(true), + )) + .collect(), + ..Default::default() + }; + save_global_settings(state.store.as_ref(), &global_settings) + .await + .unwrap(); + + // A generic provider has no profile, so it declares no STS refresh + // binding. STS must not be configurable against it. + create_provider_record( + state.store.as_ref(), + Provider { + metadata: Some(openshell_core::proto::datamodel::v1::ObjectMeta { + id: String::new(), + name: "generic-no-profile".to_string(), + created_at_ms: 0, + labels: HashMap::new(), + resource_version: 0, + }), + r#type: "generic".to_string(), + credentials: std::iter::once(( + "AWS_ACCESS_KEY_ID".to_string(), + "placeholder".to_string(), + )) + .collect(), + config: HashMap::new(), + credential_expires_at_ms: HashMap::new(), + }, + ) + .await + .unwrap(); + + let err = handle_configure_provider_refresh( + &state, + Request::new(ConfigureProviderRefreshRequest { + provider: "generic-no-profile".to_string(), + credential_key: "AWS_ACCESS_KEY_ID".to_string(), + strategy: ProviderCredentialRefreshStrategy::AwsStsAssumeRole as i32, + material: HashMap::from([( + "role_arn".to_string(), + "arn:aws:iam::123456789012:role/Test".to_string(), + )]), + secret_material_keys: Vec::new(), + expires_at_ms: None, + }), + ) + .await + .unwrap_err(); + + assert_eq!(err.code(), Code::FailedPrecondition); + assert!(err.message().contains("provider profile")); + } + + #[tokio::test] + async fn configure_aws_sts_rejects_non_canonical_credential_key() { + use crate::grpc::StoredSettingValue; + use crate::grpc::StoredSettings; + use crate::grpc::policy::save_global_settings; + + let state = test_server_state().await; + let global_settings = StoredSettings { + revision: 1, + settings: std::iter::once(( + openshell_core::settings::PROVIDERS_V2_ENABLED_KEY.to_string(), + StoredSettingValue::Bool(true), + )) + .collect(), + ..Default::default() + }; + save_global_settings(state.store.as_ref(), &global_settings) + .await + .unwrap(); + + create_provider_record( + state.store.as_ref(), + Provider { + metadata: Some(openshell_core::proto::datamodel::v1::ObjectMeta { + id: String::new(), + name: "aws-wrong-key".to_string(), + created_at_ms: 0, + labels: HashMap::new(), + resource_version: 0, + }), + r#type: "aws".to_string(), + credentials: HashMap::new(), + config: HashMap::new(), + credential_expires_at_ms: HashMap::new(), + }, + ) + .await + .unwrap(); + + // AWS_SECRET_ACCESS_KEY is a co-managed output, not the STS primary. The + // profile declares no refresh on it, so STS cannot be pinned there. + let err = handle_configure_provider_refresh( + &state, + Request::new(ConfigureProviderRefreshRequest { + provider: "aws-wrong-key".to_string(), + credential_key: "AWS_SECRET_ACCESS_KEY".to_string(), + strategy: ProviderCredentialRefreshStrategy::AwsStsAssumeRole as i32, + material: HashMap::from([( + "role_arn".to_string(), + "arn:aws:iam::123456789012:role/Test".to_string(), + )]), + secret_material_keys: Vec::new(), + expires_at_ms: None, + }), + ) + .await + .unwrap_err(); + + assert_eq!(err.code(), Code::FailedPrecondition); + assert!(err.message().contains("AWS_ACCESS_KEY_ID")); + } + #[tokio::test] async fn active_provider_environment_keys_include_additional_output_keys() { let state = test_server_state().await; diff --git a/crates/openshell-server/src/provider_refresh.rs b/crates/openshell-server/src/provider_refresh.rs index 7cf10bb893..954c48b82c 100644 --- a/crates/openshell-server/src/provider_refresh.rs +++ b/crates/openshell-server/src/provider_refresh.rs @@ -649,26 +649,22 @@ async fn mint_aws_sts_assume_role( let max_expires = now_ms + max_lifetime_i64 * 1000; let expires_at_ms = expires_at_ms.min(max_expires); - // Map STS response fields to the env keys the profile bound to each - // semantic output. Fall back to the standard AWS names when the state has - // no resolved mapping (older refresh states configured before - // additional_outputs existed). Profile validation guarantees these resolve - // to the standard names the SigV4 signer expects. + // Map STS response fields to the env keys the profile bound to each semantic + // output. Configure pins these from the profile's additional_outputs, so a + // missing mapping means the state was not configured against a valid AWS STS + // profile binding; refuse rather than guessing standard AWS names. let output_values = [ - ( - "secret_access_key", - secret_access_key, - "AWS_SECRET_ACCESS_KEY", - ), - ("session_token", session_token, "AWS_SESSION_TOKEN"), + ("secret_access_key", secret_access_key), + ("session_token", session_token), ]; let mut additional = HashMap::new(); - for (output_id, value, default_key) in output_values { - let env_key = state - .additional_output_keys - .get(output_id) - .map_or(default_key, String::as_str); - additional.insert(env_key.to_string(), value); + for (output_id, value) in output_values { + let env_key = state.additional_output_keys.get(output_id).ok_or_else(|| { + Status::failed_precondition(format!( + "refresh state missing resolved output key for '{output_id}'; reconfigure the AWS STS refresh" + )) + })?; + additional.insert(env_key.clone(), value); } Ok(MintedCredential { @@ -1370,7 +1366,13 @@ mod tests { &prov, "AWS_ACCESS_KEY_ID", NewRefreshStateConfig { - additional_output_keys: HashMap::new(), + additional_output_keys: HashMap::from([ + ( + "secret_access_key".to_string(), + "AWS_SECRET_ACCESS_KEY".to_string(), + ), + ("session_token".to_string(), "AWS_SESSION_TOKEN".to_string()), + ]), strategy: ProviderCredentialRefreshStrategy::AwsStsAssumeRole, material: HashMap::from([ ( From 29970bb42f89ab5da999e293bf0830e427c57561 Mon Sep 17 00:00:00 2001 From: Russell Bryant Date: Tue, 14 Jul 2026 15:03:31 -0400 Subject: [PATCH 05/16] fix(providers): require AWS STS source credentials as a complete pair A refresh that supplied only aws_access_key_id or only aws_secret_access_key silently ignored the lone key and fell back to the gateway's ambient AWS identity, minting against an unintended principal (CWE-20). Require both keys present or both absent, rejecting a partial pair at the configure boundary and again in the mint path. Signed-off-by: Russell Bryant --- crates/openshell-server/src/grpc/provider.rs | 81 +++++++++++++++++++ .../openshell-server/src/provider_refresh.rs | 81 ++++++++++++++++--- 2 files changed, 153 insertions(+), 9 deletions(-) diff --git a/crates/openshell-server/src/grpc/provider.rs b/crates/openshell-server/src/grpc/provider.rs index b0616005bf..6c3bfecf53 100644 --- a/crates/openshell-server/src/grpc/provider.rs +++ b/crates/openshell-server/src/grpc/provider.rs @@ -2133,6 +2133,22 @@ pub(super) async fn handle_configure_provider_refresh( "sts_endpoint_url material is not permitted", )); } + // Explicit AWS source credentials are all-or-nothing. Reject a partial pair + // early so a lone key can't later fall through to the gateway's ambient + // identity at mint time (CWE-20). + let has_source_access_key = request + .material + .get("aws_access_key_id") + .is_some_and(|value| !value.trim().is_empty()); + let has_source_secret_key = request + .material + .get("aws_secret_access_key") + .is_some_and(|value| !value.trim().is_empty()); + if has_source_access_key != has_source_secret_key { + return Err(Status::invalid_argument( + "aws_access_key_id and aws_secret_access_key must both be set or both omitted", + )); + } if request .expires_at_ms .is_some_and(|expires_at_ms| expires_at_ms < 0) @@ -6347,6 +6363,71 @@ mod tests { ); } + #[tokio::test] + async fn configure_aws_sts_rejects_partial_source_credentials() { + use crate::grpc::StoredSettingValue; + use crate::grpc::StoredSettings; + use crate::grpc::policy::save_global_settings; + + let state = test_server_state().await; + let global_settings = StoredSettings { + revision: 1, + settings: std::iter::once(( + openshell_core::settings::PROVIDERS_V2_ENABLED_KEY.to_string(), + StoredSettingValue::Bool(true), + )) + .collect(), + ..Default::default() + }; + save_global_settings(state.store.as_ref(), &global_settings) + .await + .unwrap(); + + create_provider_record( + state.store.as_ref(), + Provider { + metadata: Some(openshell_core::proto::datamodel::v1::ObjectMeta { + id: String::new(), + name: "aws-partial-source".to_string(), + created_at_ms: 0, + labels: HashMap::new(), + resource_version: 0, + }), + r#type: "aws".to_string(), + credentials: HashMap::new(), + config: HashMap::new(), + credential_expires_at_ms: HashMap::new(), + }, + ) + .await + .unwrap(); + + // aws_access_key_id without its secret half must be rejected rather than + // silently falling back to the gateway's ambient identity. + let err = handle_configure_provider_refresh( + &state, + Request::new(ConfigureProviderRefreshRequest { + provider: "aws-partial-source".to_string(), + credential_key: "AWS_ACCESS_KEY_ID".to_string(), + strategy: ProviderCredentialRefreshStrategy::AwsStsAssumeRole as i32, + material: HashMap::from([ + ( + "role_arn".to_string(), + "arn:aws:iam::123456789012:role/Test".to_string(), + ), + ("aws_access_key_id".to_string(), "AKIATESTKEY".to_string()), + ]), + secret_material_keys: Vec::new(), + expires_at_ms: None, + }), + ) + .await + .unwrap_err(); + + assert_eq!(err.code(), Code::InvalidArgument); + assert!(err.message().contains("both be set or both omitted")); + } + #[tokio::test] async fn configure_aws_sts_persists_resolved_additional_output_keys() { use crate::grpc::StoredSettingValue; diff --git a/crates/openshell-server/src/provider_refresh.rs b/crates/openshell-server/src/provider_refresh.rs index 954c48b82c..98c9a72d96 100644 --- a/crates/openshell-server/src/provider_refresh.rs +++ b/crates/openshell-server/src/provider_refresh.rs @@ -583,18 +583,29 @@ async fn mint_aws_sts_assume_role( let mut config_loader = aws_config::defaults(aws_config::BehaviorVersion::latest()).region(region_provider); - if let (Some(access_key), Some(secret_key)) = ( + // Explicit source credentials are all-or-nothing. A lone key must not + // silently fall through to the gateway's ambient identity (CWE-20): the + // caller asked for a specific principal, so an incomplete pair is an error. + match ( material_value(&state.material, &["aws_access_key_id"]), material_value(&state.material, &["aws_secret_access_key"]), ) { - let creds = aws_sdk_sts::config::Credentials::new( - access_key, - secret_key, - None, - None, - "openshell-provider-refresh", - ); - config_loader = config_loader.credentials_provider(creds); + (Some(access_key), Some(secret_key)) => { + let creds = aws_sdk_sts::config::Credentials::new( + access_key, + secret_key, + None, + None, + "openshell-provider-refresh", + ); + config_loader = config_loader.credentials_provider(creds); + } + (None, None) => {} + _ => { + return Err(Status::invalid_argument( + "aws_access_key_id and aws_secret_access_key must both be set or both omitted", + )); + } } let sdk_config = config_loader.load().await; @@ -1505,6 +1516,58 @@ mod tests { assert!(!stored.credentials.contains_key("AWS_SECRET_ACCESS_KEY")); } + #[tokio::test] + async fn aws_sts_mint_rejects_partial_source_credentials() { + let store = test_store().await; + let prov = provider("aws-sts-partial", "aws"); + store.put_message(&prov).await.unwrap(); + + // Only the access key half of the explicit source pair is present. The + // mint must fail rather than fall back to the gateway's ambient identity. + let state = new_refresh_state( + &prov, + "AWS_ACCESS_KEY_ID", + NewRefreshStateConfig { + additional_output_keys: HashMap::from([ + ( + "secret_access_key".to_string(), + "AWS_SECRET_ACCESS_KEY".to_string(), + ), + ("session_token".to_string(), "AWS_SESSION_TOKEN".to_string()), + ]), + strategy: ProviderCredentialRefreshStrategy::AwsStsAssumeRole, + material: HashMap::from([ + ( + "role_arn".to_string(), + "arn:aws:iam::123456789012:role/TestRole".to_string(), + ), + ("aws_access_key_id".to_string(), "AKIATESTKEY".to_string()), + ]), + secret_material_keys: Vec::new(), + expires_at_ms: 0, + token_url: String::new(), + scopes: Vec::new(), + refresh_before_seconds: 300, + max_lifetime_seconds: 3600, + }, + ) + .unwrap(); + put_refresh_state(&store, &state).await.unwrap(); + + let err = refresh_provider_credential(&store, "aws-sts-partial", "AWS_ACCESS_KEY_ID") + .await + .unwrap_err(); + assert_eq!(err.code(), tonic::Code::InvalidArgument); + assert!(err.message().contains("both be set or both omitted")); + + let stored = store + .get_message_by_name::("aws-sts-partial") + .await + .unwrap() + .unwrap(); + assert!(!stored.credentials.contains_key("AWS_ACCESS_KEY_ID")); + } + #[tokio::test] async fn apply_minted_credential_writes_additional_keys() { use super::apply_minted_credential; From a726dfce13e1fdadbe23f0a70776244899435e68 Mon Sep 17 00:00:00 2001 From: Russell Bryant Date: Tue, 14 Jul 2026 15:09:22 -0400 Subject: [PATCH 06/16] fix(providers): enforce providers_v2 gate on every AWS STS mint The providers_v2_enabled gate for aws_sts_assume_role was checked only in handle_configure_provider_refresh. Once a refresh was configured, the worker sweep and manual rotation kept minting STS credentials even after the setting was disabled. Enforce the gate in refresh_provider_credential, the shared execution path for both the worker and manual rotation, so disabling providers_v2_enabled halts further mints. The rejection is recorded on the refresh state for observability. Signed-off-by: Russell Bryant --- crates/openshell-server/src/grpc/policy.rs | 24 +++++ crates/openshell-server/src/grpc/provider.rs | 92 +++++++++++++++++++ .../openshell-server/src/provider_refresh.rs | 67 ++++++++++++++ 3 files changed, 183 insertions(+) diff --git a/crates/openshell-server/src/grpc/policy.rs b/crates/openshell-server/src/grpc/policy.rs index 563f720c77..945fed20c8 100644 --- a/crates/openshell-server/src/grpc/policy.rs +++ b/crates/openshell-server/src/grpc/policy.rs @@ -3745,6 +3745,30 @@ pub(super) async fn load_global_settings(store: &Store) -> Result Result { + let global_settings = load_global_settings(store).await?; + bool_setting_enabled(&global_settings, key) +} + +/// Test helper: set a boolean global setting, loading current settings first so +/// the CAS write succeeds whether the record already exists or not. Available to +/// sibling test modules without exposing the private `StoredSettings` type. +#[cfg(test)] +pub async fn set_global_bool_setting_for_test( + store: &Store, + key: &str, + value: bool, +) -> Result<(), Status> { + let mut settings = load_global_settings(store).await?; + settings + .settings + .insert(key.to_string(), StoredSettingValue::Bool(value)); + save_global_settings(store, &settings).await +} + pub(super) async fn save_global_settings( store: &Store, settings: &StoredSettings, diff --git a/crates/openshell-server/src/grpc/provider.rs b/crates/openshell-server/src/grpc/provider.rs index 6c3bfecf53..043ce10de3 100644 --- a/crates/openshell-server/src/grpc/provider.rs +++ b/crates/openshell-server/src/grpc/provider.rs @@ -6637,6 +6637,98 @@ mod tests { assert!(err.message().contains("AWS_ACCESS_KEY_ID")); } + #[tokio::test] + async fn rotate_aws_sts_blocked_after_providers_v2_disabled() { + use crate::grpc::policy::set_global_bool_setting_for_test; + + let state = test_server_state().await; + + set_global_bool_setting_for_test( + state.store.as_ref(), + openshell_core::settings::PROVIDERS_V2_ENABLED_KEY, + true, + ) + .await + .unwrap(); + + create_provider_record( + state.store.as_ref(), + Provider { + metadata: Some(openshell_core::proto::datamodel::v1::ObjectMeta { + id: String::new(), + name: "aws-gate".to_string(), + created_at_ms: 0, + labels: HashMap::new(), + resource_version: 0, + }), + r#type: "aws".to_string(), + credentials: HashMap::new(), + config: HashMap::new(), + credential_expires_at_ms: HashMap::new(), + }, + ) + .await + .unwrap(); + + handle_configure_provider_refresh( + &state, + Request::new(ConfigureProviderRefreshRequest { + provider: "aws-gate".to_string(), + credential_key: "AWS_ACCESS_KEY_ID".to_string(), + strategy: ProviderCredentialRefreshStrategy::AwsStsAssumeRole as i32, + material: HashMap::from([( + "role_arn".to_string(), + "arn:aws:iam::123456789012:role/Test".to_string(), + )]), + secret_material_keys: Vec::new(), + expires_at_ms: None, + }), + ) + .await + .unwrap(); + + // Disable the gate after the refresh is already configured. + set_global_bool_setting_for_test( + state.store.as_ref(), + openshell_core::settings::PROVIDERS_V2_ENABLED_KEY, + false, + ) + .await + .unwrap(); + + let err = handle_rotate_provider_credential( + &state, + Request::new(RotateProviderCredentialRequest { + provider: "aws-gate".to_string(), + credential_key: "AWS_ACCESS_KEY_ID".to_string(), + }), + ) + .await + .unwrap_err(); + + assert_eq!(err.code(), Code::FailedPrecondition); + assert!(err.message().contains("providers_v2_enabled")); + + // The gate rejection is recorded on the refresh state, and no credential + // was minted into the provider. + let provider = state + .store + .get_message_by_name::("aws-gate") + .await + .unwrap() + .unwrap(); + assert!(!provider.credentials.contains_key("AWS_ACCESS_KEY_ID")); + let refresh_state = crate::provider_refresh::get_refresh_state( + state.store.as_ref(), + provider.object_id(), + "AWS_ACCESS_KEY_ID", + ) + .await + .unwrap() + .expect("refresh state should exist"); + assert_eq!(refresh_state.status, "error"); + } + #[tokio::test] async fn active_provider_environment_keys_include_additional_output_keys() { let state = test_server_state().await; diff --git a/crates/openshell-server/src/provider_refresh.rs b/crates/openshell-server/src/provider_refresh.rs index 98c9a72d96..5a764f4ae7 100644 --- a/crates/openshell-server/src/provider_refresh.rs +++ b/crates/openshell-server/src/provider_refresh.rs @@ -317,6 +317,27 @@ pub async fn refresh_provider_credential( "provider credential refresh started" ); + // Enforce the providers_v2 gate on every mint, not just at configure time. + // Otherwise disabling providers_v2_enabled leaves already-configured refresh + // states that the worker and manual rotation keep minting from. + if let Err(err) = ensure_refresh_providers_v2_gate(store, &state).await { + let now_ms = current_time_ms(); + state.status = "error".to_string(); + state.last_error = err.message().to_string(); + state.next_refresh_at_ms = + now_ms.saturating_add(REFRESH_ERROR_RETRY_SECONDS.saturating_mul(1000)); + put_refresh_state(store, &state).await?; + warn!( + provider = %state.provider_name, + credential_key = %state.credential_key, + strategy = %refresh_strategy_name(state.strategy), + status = %state.status, + error = %err, + "provider credential refresh gate rejected" + ); + return Err(err); + } + match mint_credential(&state).await { Ok(minted) => { let now_ms = current_time_ms(); @@ -456,6 +477,31 @@ async fn apply_minted_credential( .map_err(|e| Status::internal(format!("persist refreshed provider credential failed: {e}"))) } +/// Reject minting for strategies that require `providers_v2_enabled` when the +/// setting is off. Runs on every refresh (worker sweep and manual rotation), so +/// disabling the setting halts further mints from already-configured states. +async fn ensure_refresh_providers_v2_gate( + store: &Store, + state: &StoredProviderCredentialRefreshState, +) -> Result<(), Status> { + let strategy = ProviderCredentialRefreshStrategy::try_from(state.strategy) + .unwrap_or(ProviderCredentialRefreshStrategy::Unspecified); + if strategy != ProviderCredentialRefreshStrategy::AwsStsAssumeRole { + return Ok(()); + } + if !crate::grpc::policy::global_bool_setting_enabled( + store, + openshell_core::settings::PROVIDERS_V2_ENABLED_KEY, + ) + .await? + { + return Err(Status::failed_precondition( + "aws_sts_assume_role requires providers_v2_enabled=true", + )); + } + Ok(()) +} + async fn mint_credential( state: &StoredProviderCredentialRefreshState, ) -> Result { @@ -1370,6 +1416,13 @@ mod tests { .await; let store = test_store().await; + crate::grpc::policy::set_global_bool_setting_for_test( + &store, + openshell_core::settings::PROVIDERS_V2_ENABLED_KEY, + true, + ) + .await + .unwrap(); let prov = provider("aws-sts-test", "aws"); store.put_message(&prov).await.unwrap(); @@ -1455,6 +1508,13 @@ mod tests { .await; let store = test_store().await; + crate::grpc::policy::set_global_bool_setting_for_test( + &store, + openshell_core::settings::PROVIDERS_V2_ENABLED_KEY, + true, + ) + .await + .unwrap(); let prov = provider("aws-sts-custom", "aws"); store.put_message(&prov).await.unwrap(); @@ -1519,6 +1579,13 @@ mod tests { #[tokio::test] async fn aws_sts_mint_rejects_partial_source_credentials() { let store = test_store().await; + crate::grpc::policy::set_global_bool_setting_for_test( + &store, + openshell_core::settings::PROVIDERS_V2_ENABLED_KEY, + true, + ) + .await + .unwrap(); let prov = provider("aws-sts-partial", "aws"); store.put_message(&prov).await.unwrap(); From 6799dcc4624599b11d0e07c7fddf6b1f75f51fb3 Mon Sep 17 00:00:00 2001 From: Russell Bryant Date: Tue, 14 Jul 2026 15:11:06 -0400 Subject: [PATCH 07/16] fix(providers): clear all refresh-owned expiries when deleting a refresh Deleting an AWS STS refresh cleared only the primary credential's expiry, leaving stale refresh-owned expiries on the co-managed AWS_SECRET_ACCESS_KEY and AWS_SESSION_TOKEN credentials. Clear the expiry for the primary and every pinned additional output whose stored expiry still matches the refresh's, in a single provider update. Expiries that were independently changed (no longer matching the refresh) are left untouched. Signed-off-by: Russell Bryant --- crates/openshell-server/src/grpc/provider.rs | 150 +++++++++++++++++-- 1 file changed, 136 insertions(+), 14 deletions(-) diff --git a/crates/openshell-server/src/grpc/provider.rs b/crates/openshell-server/src/grpc/provider.rs index 043ce10de3..40820455ad 100644 --- a/crates/openshell-server/src/grpc/provider.rs +++ b/crates/openshell-server/src/grpc/provider.rs @@ -2351,16 +2351,31 @@ pub(super) async fn handle_delete_provider_refresh( ) .await?; - let refresh_owned_expiry = existing_refresh_state - .as_ref() - .is_some_and(|refresh_state| { - refresh_state.expires_at_ms > 0 - && provider - .credential_expires_at_ms - .get(credential_key) - .is_some_and(|expires_at_ms| *expires_at_ms == refresh_state.expires_at_ms) - }); - if refresh_owned_expiry { + // A refresh co-manages the expiry of its primary credential and every pinned + // additional output. Clear each one this refresh owned (its stored expiry + // still matches the refresh's), but leave independently updated expiries in + // place. Doing it in one provider update keeps the record consistent. + let mut cleared_expiries = std::collections::HashMap::new(); + if let Some(refresh_state) = existing_refresh_state.as_ref() + && refresh_state.expires_at_ms > 0 + { + let owned_keys = std::iter::once(credential_key).chain( + refresh_state + .additional_output_keys + .values() + .map(String::as_str), + ); + for key in owned_keys { + if provider + .credential_expires_at_ms + .get(key) + .is_some_and(|expires_at_ms| *expires_at_ms == refresh_state.expires_at_ms) + { + cleared_expiries.insert(key.to_string(), 0); + } + } + } + if !cleared_expiries.is_empty() { let updated = Provider { metadata: Some(openshell_core::proto::datamodel::v1::ObjectMeta { id: String::new(), @@ -2372,10 +2387,7 @@ pub(super) async fn handle_delete_provider_refresh( r#type: String::new(), credentials: std::collections::HashMap::new(), config: std::collections::HashMap::new(), - credential_expires_at_ms: std::collections::HashMap::from([( - credential_key.to_string(), - 0, - )]), + credential_expires_at_ms: cleared_expiries, }; update_provider_record(state.store.as_ref(), updated).await?; } @@ -4011,6 +4023,116 @@ mod tests { ); } + #[tokio::test] + async fn delete_aws_sts_refresh_clears_all_pinned_output_expiries() { + use crate::grpc::policy::set_global_bool_setting_for_test; + + let state = test_server_state().await; + set_global_bool_setting_for_test( + state.store.as_ref(), + openshell_core::settings::PROVIDERS_V2_ENABLED_KEY, + true, + ) + .await + .unwrap(); + + create_provider_record( + state.store.as_ref(), + Provider { + metadata: Some(openshell_core::proto::datamodel::v1::ObjectMeta { + id: String::new(), + name: "aws-delete".to_string(), + created_at_ms: 0, + labels: HashMap::new(), + resource_version: 0, + }), + r#type: "aws".to_string(), + credentials: HashMap::new(), + config: HashMap::new(), + credential_expires_at_ms: HashMap::new(), + }, + ) + .await + .unwrap(); + + let refresh_expires_at_ms = crate::persistence::current_time_ms() + 60_000; + handle_configure_provider_refresh( + &state, + Request::new(ConfigureProviderRefreshRequest { + provider: "aws-delete".to_string(), + credential_key: "AWS_ACCESS_KEY_ID".to_string(), + strategy: ProviderCredentialRefreshStrategy::AwsStsAssumeRole as i32, + material: HashMap::from([( + "role_arn".to_string(), + "arn:aws:iam::123456789012:role/Test".to_string(), + )]), + secret_material_keys: Vec::new(), + expires_at_ms: Some(refresh_expires_at_ms), + }), + ) + .await + .unwrap(); + + // Simulate a completed mint: the primary and secret share the refresh's + // expiry (refresh-owned), while the session token was independently + // updated to a later value that must survive the delete. + let independent_expires_at_ms = refresh_expires_at_ms + 120_000; + update_provider_record( + state.store.as_ref(), + Provider { + metadata: Some(openshell_core::proto::datamodel::v1::ObjectMeta { + id: String::new(), + name: "aws-delete".to_string(), + created_at_ms: 0, + labels: HashMap::new(), + resource_version: 0, + }), + r#type: String::new(), + credentials: HashMap::new(), + config: HashMap::new(), + credential_expires_at_ms: HashMap::from([ + ("AWS_SECRET_ACCESS_KEY".to_string(), refresh_expires_at_ms), + ("AWS_SESSION_TOKEN".to_string(), independent_expires_at_ms), + ]), + }, + ) + .await + .unwrap(); + + handle_delete_provider_refresh( + &state, + Request::new(DeleteProviderRefreshRequest { + provider: "aws-delete".to_string(), + credential_key: "AWS_ACCESS_KEY_ID".to_string(), + }), + ) + .await + .unwrap(); + + let provider = state + .store + .get_message_by_name::("aws-delete") + .await + .unwrap() + .unwrap(); + // Refresh-owned expiries for the primary and secret are cleared. + assert!( + !provider + .credential_expires_at_ms + .contains_key("AWS_ACCESS_KEY_ID") + ); + assert!( + !provider + .credential_expires_at_ms + .contains_key("AWS_SECRET_ACCESS_KEY") + ); + // The independently updated session-token expiry is preserved. + assert_eq!( + provider.credential_expires_at_ms.get("AWS_SESSION_TOKEN"), + Some(&independent_expires_at_ms) + ); + } + #[tokio::test] async fn configure_provider_refresh_rejects_credential_key_collision_for_attached_sandbox() { let state = test_server_state().await; From e8495fdf0b81e49853fa6a4533095bacdec5f298 Mon Sep 17 00:00:00 2001 From: Russell Bryant Date: Tue, 14 Jul 2026 15:11:45 -0400 Subject: [PATCH 08/16] docs(providers): read AWS STS secret from env, not process arguments The off-AWS AWS STS refresh example expanded $AWS_SECRET_ACCESS_KEY into a --material argument, exposing the long-lived secret in the host process table (CWE-200). Use --secret-material-env aws_secret_access_key=AWS_SECRET_ACCESS_KEY so the CLI reads the value from its own environment and marks it secret. Signed-off-by: Russell Bryant --- docs/sandboxes/manage-providers.mdx | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/docs/sandboxes/manage-providers.mdx b/docs/sandboxes/manage-providers.mdx index 218fa697b0..2ea77dd1f3 100644 --- a/docs/sandboxes/manage-providers.mdx +++ b/docs/sandboxes/manage-providers.mdx @@ -211,10 +211,13 @@ openshell provider refresh configure my-aws \ --strategy aws-sts-assume-role \ --material role_arn="arn:aws:iam::123456789012:role/SandboxS3Writer" \ --material aws_access_key_id="$AWS_ACCESS_KEY_ID" \ - --material aws_secret_access_key="$AWS_SECRET_ACCESS_KEY" \ - --secret-material-key aws_secret_access_key + --secret-material-env aws_secret_access_key=AWS_SECRET_ACCESS_KEY ``` +Pass the long-lived secret with `--secret-material-env` so the CLI reads it +from its own environment instead of expanding it into a process argument, +where it would be visible in the host process table. + Use the generic `aws` profile type for multi-service access and scope endpoints via sandbox network policy. Use `aws-s3` for S3-specific endpoint rules. From 557221f32124dea0040642b127078b0f6f6bf4be Mon Sep 17 00:00:00 2001 From: Russell Bryant Date: Tue, 14 Jul 2026 16:08:23 -0400 Subject: [PATCH 09/16] fix(providers): decide refresh-owned expiry cleanup inside the CAS closure Deleting a refresh decided which expiries to clear from a provider snapshot read at the top of the handler, then applied an unconditional zero to each. A rotation or provider update landing between that read and the write was silently clobbered (CWE-362). Move the equality check and removal into a pure helper run inside the provider CAS closure, so the decision reflects the current stored provider rather than a stale snapshot. Only expiries that still match the deleted refresh are cleared; concurrently changed values are preserved. Signed-off-by: Russell Bryant --- crates/openshell-server/src/grpc/provider.rs | 136 ++++++++++++++----- 1 file changed, 100 insertions(+), 36 deletions(-) diff --git a/crates/openshell-server/src/grpc/provider.rs b/crates/openshell-server/src/grpc/provider.rs index 40820455ad..7f854415c9 100644 --- a/crates/openshell-server/src/grpc/provider.rs +++ b/crates/openshell-server/src/grpc/provider.rs @@ -2319,6 +2319,34 @@ pub(super) async fn handle_rotate_provider_credential( })) } +/// Remove the expiry for each key the refresh still owns — one whose stored +/// value equals the refresh's `expires_at_ms`. Independently updated expiries (no +/// longer matching) are left in place. +/// +/// This operates on `provider` in place so it can run inside a CAS closure +/// against the current stored provider. The equality check must see the latest +/// state, not a snapshot read earlier in the handler: a concurrent rotation or +/// provider update between the read and the write could otherwise be clobbered +/// by an unconditional zero (CWE-362). +fn clear_refresh_owned_expiries( + provider: &mut Provider, + refresh_expires_at_ms: i64, + owned_keys: &[String], +) { + if refresh_expires_at_ms <= 0 { + return; + } + for key in owned_keys { + if provider + .credential_expires_at_ms + .get(key) + .is_some_and(|expires_at_ms| *expires_at_ms == refresh_expires_at_ms) + { + provider.credential_expires_at_ms.remove(key); + } + } +} + pub(super) async fn handle_delete_provider_refresh( state: &Arc, request: Request, @@ -2352,44 +2380,29 @@ pub(super) async fn handle_delete_provider_refresh( .await?; // A refresh co-manages the expiry of its primary credential and every pinned - // additional output. Clear each one this refresh owned (its stored expiry - // still matches the refresh's), but leave independently updated expiries in - // place. Doing it in one provider update keeps the record consistent. - let mut cleared_expiries = std::collections::HashMap::new(); - if let Some(refresh_state) = existing_refresh_state.as_ref() + // additional output. Clear each expiry this refresh still owns, leaving + // independently updated ones in place. The equality check and removal run + // inside the CAS closure so they see the current stored provider — deciding + // from the snapshot read above would let a concurrent rotation or provider + // update land between the read and the write and then be clobbered (CWE-362). + if let Some(refresh_state) = existing_refresh_state && refresh_state.expires_at_ms > 0 { - let owned_keys = std::iter::once(credential_key).chain( - refresh_state - .additional_output_keys - .values() - .map(String::as_str), - ); - for key in owned_keys { - if provider - .credential_expires_at_ms - .get(key) - .is_some_and(|expires_at_ms| *expires_at_ms == refresh_state.expires_at_ms) - { - cleared_expiries.insert(key.to_string(), 0); - } - } - } - if !cleared_expiries.is_empty() { - let updated = Provider { - metadata: Some(openshell_core::proto::datamodel::v1::ObjectMeta { - id: String::new(), - name: provider_name.to_string(), - created_at_ms: 0, - labels: std::collections::HashMap::new(), - resource_version: 0, - }), - r#type: String::new(), - credentials: std::collections::HashMap::new(), - config: std::collections::HashMap::new(), - credential_expires_at_ms: cleared_expiries, - }; - update_provider_record(state.store.as_ref(), updated).await?; + let refresh_expires_at_ms = refresh_state.expires_at_ms; + let owned_keys: Vec = std::iter::once(credential_key.to_string()) + .chain(refresh_state.additional_output_keys.into_values()) + .collect(); + state + .store + .update_message_cas::(provider.object_id(), 0, |current| { + clear_refresh_owned_expiries(current, refresh_expires_at_ms, &owned_keys); + }) + .await + .map_err(|e| { + Status::internal(format!( + "clear refresh-owned credential expiries failed: {e}" + )) + })?; } Ok(Response::new(DeleteProviderRefreshResponse { @@ -4133,6 +4146,57 @@ mod tests { ); } + #[test] + fn clear_refresh_owned_expiries_preserves_concurrently_changed_expiry() { + // Regression for the delete-refresh TOCTOU: the clearing decision runs + // inside the CAS closure against the current provider. Simulate a + // concurrent rotation that updated AWS_SESSION_TOKEN's expiry after the + // handler's initial read but before its write — by the time the closure + // runs, that value no longer matches the deleted refresh's expiry and + // must survive, while the still-owned keys are cleared. + let refresh_expires_at_ms = 1_000_000; + let concurrently_changed = refresh_expires_at_ms + 500_000; + let mut provider = Provider { + metadata: Some(openshell_core::proto::datamodel::v1::ObjectMeta { + id: "p".to_string(), + name: "p".to_string(), + created_at_ms: 0, + labels: HashMap::new(), + resource_version: 0, + }), + r#type: "aws".to_string(), + credentials: HashMap::new(), + config: HashMap::new(), + credential_expires_at_ms: HashMap::from([ + ("AWS_ACCESS_KEY_ID".to_string(), refresh_expires_at_ms), + ("AWS_SECRET_ACCESS_KEY".to_string(), refresh_expires_at_ms), + ("AWS_SESSION_TOKEN".to_string(), concurrently_changed), + ]), + }; + let owned_keys = vec![ + "AWS_ACCESS_KEY_ID".to_string(), + "AWS_SECRET_ACCESS_KEY".to_string(), + "AWS_SESSION_TOKEN".to_string(), + ]; + + clear_refresh_owned_expiries(&mut provider, refresh_expires_at_ms, &owned_keys); + + assert!( + !provider + .credential_expires_at_ms + .contains_key("AWS_ACCESS_KEY_ID") + ); + assert!( + !provider + .credential_expires_at_ms + .contains_key("AWS_SECRET_ACCESS_KEY") + ); + assert_eq!( + provider.credential_expires_at_ms.get("AWS_SESSION_TOKEN"), + Some(&concurrently_changed) + ); + } + #[tokio::test] async fn configure_provider_refresh_rejects_credential_key_collision_for_attached_sandbox() { let state = test_server_state().await; From 6c58d2af3bcdda97cc7ecba8b7f53983c7d92daa Mon Sep 17 00:00:00 2001 From: Russell Bryant Date: Tue, 14 Jul 2026 16:09:04 -0400 Subject: [PATCH 10/16] docs(providers): give the aws-s3 STS example PyPI egress for boto3 The boto3 walkthrough ran pip install inside a sandbox attached only to the aws-s3 profile, which permits S3 egress but not PyPI, so the install would be blocked. Attach the built-in pypi profile so pip can reach PyPI, and note the custom-image alternative. Also delete the s3-curl sandbox (and the pypi provider) in the cleanup section. Signed-off-by: Russell Bryant --- examples/aws-s3-sts.md | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/examples/aws-s3-sts.md b/examples/aws-s3-sts.md index 7e41995077..e622e71117 100644 --- a/examples/aws-s3-sts.md +++ b/examples/aws-s3-sts.md @@ -158,9 +158,16 @@ The status should show `refreshed` with an expiry ~1 hour from now. ### Using boto3 (Python) +boto3 is not in the base sandbox image, and the `aws-s3` profile allows only S3 +egress. Attach the built-in `pypi` profile so `pip` can reach PyPI (or bake +boto3 into a custom sandbox image): + ```shell +openshell provider create --name pypi --type pypi --runtime-credentials + openshell sandbox create --name s3-smoke \ --provider s3-test \ + --provider pypi \ -- bash -c ' export AWS_CA_BUNDLE=/etc/openshell-tls/ca-bundle.pem pip install boto3 -q 2>&1 | tail -1 @@ -249,8 +256,10 @@ in the environment. ## 6. Clean up ```shell -# Delete the sandbox +# Delete the sandboxes and the pypi provider openshell sandbox delete s3-smoke +openshell sandbox delete s3-curl +openshell provider delete pypi # Delete AWS resources aws s3 rb "s3://${BUCKET}" --force From 623512efc2e50e133e63b9b96e4c4f28d8d97dd9 Mon Sep 17 00:00:00 2001 From: Russell Bryant Date: Tue, 14 Jul 2026 16:13:10 -0400 Subject: [PATCH 11/16] fix(prover): match whole middle-label host wildcards like the runtime The prover's host-pattern overlap only wildcard-matched the first DNS label and compared the remaining labels literally, so it missed overlap for the whole middle-label wildcards this PR added to the aws-s3 profile (e.g. *.s3.*.amazonaws.com). It therefore under-reported credential-associated policy risk (CWE-693). Match label-for-label instead: a * (or partial * glob) matches exactly one label in any position, literal labels compare literally, and a leading ** still matches one or more labels. Cover matching and non-matching label counts in tests. Signed-off-by: Russell Bryant --- crates/openshell-prover/src/credentials.rs | 71 +++++++++++++++++----- 1 file changed, 56 insertions(+), 15 deletions(-) diff --git a/crates/openshell-prover/src/credentials.rs b/crates/openshell-prover/src/credentials.rs index 586d0fbbf9..6c9a1a74f3 100644 --- a/crates/openshell-prover/src/credentials.rs +++ b/crates/openshell-prover/src/credentials.rs @@ -186,26 +186,39 @@ fn host_pattern_covers(pattern: &str, host: &str) -> bool { return false; }; + // A leading `**` matches one or more leading labels; the remaining labels + // match the tail of the host label-for-label, mirroring the runtime. if first_pattern_label == "**" { let suffix = &pattern_labels[1..]; - let host_suffix = host_labels - .len() - .checked_sub(suffix.len()) - .map(|start| &host_labels[start..]); - return !suffix.is_empty() - && host_labels.len() > suffix.len() - && matches!(host_suffix, Some(host_suffix) if host_suffix == suffix); + if suffix.is_empty() || host_labels.len() <= suffix.len() { + return false; + } + let start = host_labels.len() - suffix.len(); + return labels_match(suffix, &host_labels[start..]); } - if !first_pattern_label.contains('*') { - return false; - } + // Otherwise the pattern matches exactly one label per position. A `*` (or a + // partial `*` glob) matches a single label, literal labels must be equal, and + // wildcards may appear in any label — not just the first — to match the + // runtime's whole-middle-label wildcards (e.g. `*.s3.*.amazonaws.com`). + labels_match(&pattern_labels, &host_labels) +} - // Runtime host wildcards only apply in the first DNS label. Wildcards in - // later labels are not treated as policy globs here. +/// Match `pattern_labels` against `host_labels` positionally: equal label counts, +/// each `*`-bearing pattern label matched as a single-label glob and every other +/// label compared literally. +fn labels_match(pattern_labels: &[&str], host_labels: &[&str]) -> bool { pattern_labels.len() == host_labels.len() - && pattern_labels[1..] == host_labels[1..] - && wildcard_label_matches(first_pattern_label, host_labels[0]) + && pattern_labels + .iter() + .zip(host_labels) + .all(|(pattern, host)| { + if pattern.contains('*') { + wildcard_label_matches(pattern, host) + } else { + pattern == host + } + }) } fn wildcard_label_matches(pattern: &str, label: &str) -> bool { @@ -419,10 +432,38 @@ mod tests { "api-*.github.com", "uploads.github.com" )); - assert!(!host_patterns_overlap( + } + + #[test] + fn host_patterns_overlap_matches_whole_middle_label_wildcard() { + // A `*` matches exactly one label in any position, mirroring the runtime + // (e.g. the aws-s3 profile's `*.s3.*.amazonaws.com`). + assert!(host_patterns_overlap( "api.*.github.com", "api.v3.github.com" )); + assert!(host_patterns_overlap( + "*.s3.*.amazonaws.com", + "bucket.s3.us-east-1.amazonaws.com" + )); + assert!(host_patterns_overlap( + "s3.*.amazonaws.com", + "s3.eu-west-1.amazonaws.com" + )); + // A single `*` label matches exactly one label — not zero, not several. + assert!(!host_patterns_overlap( + "s3.*.amazonaws.com", + "s3.amazonaws.com" + )); + assert!(!host_patterns_overlap( + "*.s3.*.amazonaws.com", + "bucket.s3.dualstack.us-east-1.amazonaws.com" + )); + // A literal middle label must still match literally. + assert!(!host_patterns_overlap( + "api.*.github.com", + "api.v3.gitlab.com" + )); } #[test] From f4627754fc889301762dffbdb0571d2ed34b40ed Mon Sep 17 00:00:00 2001 From: Russell Bryant Date: Tue, 14 Jul 2026 16:34:51 -0400 Subject: [PATCH 12/16] fix(providers): don't let a rotation resurrect a deleted refresh refresh_provider_credential read the refresh state, awaited the STS (or OAuth) request, then wrote credentials and persisted state with an unconditional upsert. A delete-refresh completing while the request was in flight was silently undone: the rotation re-minted credentials and recreated the deleted refresh state, including stored source-credential material (CWE-362). Capture the refresh generation at the start and make every terminal persist a version-matched UPDATE (never an insert), so a deleted or superseded refresh is detected and never recreated. Re-check that the refresh still exists after minting, before writing credentials into the provider, so a removed refresh is not re-minted. Add a deterministic regression test that pauses mock STS, deletes the refresh, releases STS, and verifies the refresh state and provider credentials are unchanged. Signed-off-by: Russell Bryant --- .../openshell-server/src/provider_refresh.rs | 219 +++++++++++++++++- 1 file changed, 212 insertions(+), 7 deletions(-) diff --git a/crates/openshell-server/src/provider_refresh.rs b/crates/openshell-server/src/provider_refresh.rs index 5a764f4ae7..aa6a98ff54 100644 --- a/crates/openshell-server/src/provider_refresh.rs +++ b/crates/openshell-server/src/provider_refresh.rs @@ -5,7 +5,7 @@ #![allow(clippy::result_large_err)] -use crate::persistence::{ObjectType, Store, current_time_ms}; +use crate::persistence::{ObjectType, PersistenceError, Store, WriteCondition, current_time_ms}; use openshell_core::proto::{ Provider, ProviderCredentialRefreshStatus, ProviderCredentialRefreshStrategy, StoredProviderCredentialRefreshState, @@ -47,6 +47,41 @@ pub async fn put_refresh_state( .map_err(|e| Status::internal(format!("persist provider refresh state failed: {e}"))) } +/// Persist an updated refresh state only if the row still exists with the +/// generation read at the start of the rotation. +/// +/// Uses a version-matched UPDATE, which never inserts — so a refresh deleted +/// while an STS (or OAuth) request was in flight is not resurrected, and its +/// stored source-credential material is not recreated (CWE-362). Returns `true` +/// when persisted, `false` when the refresh was deleted or superseded by a +/// concurrent write (in which case nothing was written). +async fn persist_refresh_state_if_current( + store: &Store, + state: &StoredProviderCredentialRefreshState, + expected_version: u64, +) -> Result { + match store + .put_if( + StoredProviderCredentialRefreshState::object_type(), + state.object_id(), + state.object_name(), + &state.encode_to_vec(), + None, + WriteCondition::MatchResourceVersion(expected_version), + ) + .await + { + Ok(_) => Ok(true), + // Version mismatch (superseded) or the row is gone (deleted): the + // version-matched UPDATE matched no row, so nothing was written. + Err(PersistenceError::Conflict { .. }) => Ok(false), + Err(PersistenceError::Database(message)) if message.contains("not found") => Ok(false), + Err(e) => Err(Status::internal(format!( + "persist provider refresh state failed: {e}" + ))), + } +} + pub async fn list_refresh_states_for_provider( store: &Store, provider_id: &str, @@ -306,6 +341,13 @@ pub async fn refresh_provider_credential( else { return Err(Status::not_found("provider refresh state not found")); }; + // Generation of the refresh at the start of the rotation. Terminal persists + // match on it so a concurrent delete or rotation is detected rather than + // clobbered, and a deleted refresh is never recreated (CWE-362). + let expected_version = state + .metadata + .as_ref() + .map_or(0, |meta| meta.resource_version); info!( provider = %state.provider_name, @@ -326,7 +368,7 @@ pub async fn refresh_provider_credential( state.last_error = err.message().to_string(); state.next_refresh_at_ms = now_ms.saturating_add(REFRESH_ERROR_RETRY_SECONDS.saturating_mul(1000)); - put_refresh_state(store, &state).await?; + persist_refresh_state_if_current(store, &state, expected_version).await?; warn!( provider = %state.provider_name, credential_key = %state.credential_key, @@ -341,6 +383,26 @@ pub async fn refresh_provider_credential( match mint_credential(&state).await { Ok(minted) => { let now_ms = current_time_ms(); + // A delete-refresh can complete while the mint request is in flight. + // Verify the refresh still exists before writing minted credentials + // into the provider, so a removed refresh is not silently re-minted + // (CWE-362). The version-matched persist below is the authoritative + // guard against resurrecting deleted state; this check just avoids + // writing credentials we are about to discard. + if get_refresh_state(store, provider.object_id(), credential_key) + .await? + .is_none() + { + warn!( + provider = %state.provider_name, + credential_key = %state.credential_key, + strategy = %refresh_strategy_name(state.strategy), + "provider credential refresh deleted during rotation; discarding minted credentials" + ); + return Err(Status::aborted( + "provider refresh was deleted during rotation", + )); + } if let Err(err) = apply_minted_credential(store, &provider, credential_key, &minted).await { @@ -348,7 +410,7 @@ pub async fn refresh_provider_credential( state.last_error = err.message().to_string(); state.next_refresh_at_ms = now_ms.saturating_add(REFRESH_ERROR_RETRY_SECONDS.saturating_mul(1000)); - put_refresh_state(store, &state).await?; + persist_refresh_state_if_current(store, &state, expected_version).await?; warn!( provider = %state.provider_name, credential_key = %state.credential_key, @@ -383,7 +445,17 @@ pub async fn refresh_provider_credential( state.last_refresh_at_ms = now_ms; state.status = "refreshed".to_string(); state.last_error.clear(); - put_refresh_state(store, &state).await?; + if !persist_refresh_state_if_current(store, &state, expected_version).await? { + warn!( + provider = %state.provider_name, + credential_key = %state.credential_key, + strategy = %refresh_strategy_name(state.strategy), + "provider credential refresh deleted or superseded during rotation; not resurrecting refresh state" + ); + return Err(Status::aborted( + "provider refresh was deleted during rotation", + )); + } info!( provider = %state.provider_name, credential_key = %state.credential_key, @@ -402,7 +474,7 @@ pub async fn refresh_provider_credential( state.last_error = err.message().to_string(); state.next_refresh_at_ms = now_ms.saturating_add(REFRESH_ERROR_RETRY_SECONDS.saturating_mul(1000)); - put_refresh_state(store, &state).await?; + persist_refresh_state_if_current(store, &state, expected_version).await?; warn!( provider = %state.provider_name, credential_key = %state.credential_key, @@ -982,8 +1054,8 @@ async fn run_refresh_worker_tick(store: &Store) -> Result<(), Status> { #[cfg(test)] mod tests { use super::{ - NewRefreshStateConfig, get_refresh_state, new_refresh_state, put_refresh_state, - refresh_provider_credential, refresh_state_name, refresh_strategy_name, + NewRefreshStateConfig, delete_refresh_state, get_refresh_state, new_refresh_state, + put_refresh_state, refresh_provider_credential, refresh_state_name, refresh_strategy_name, run_refresh_worker_tick, seconds_until_ms, }; use crate::persistence::test_store; @@ -1749,6 +1821,139 @@ mod tests { assert!(err.message().contains("AWS_SECRET_ACCESS_KEY")); } + // A wiremock responder that blocks the STS response until the test releases + // it, so a delete-refresh can be interleaved deterministically while the + // rotation is parked awaiting STS. + struct GatedStsResponder { + hit: std::sync::Mutex>>, + release: std::sync::Mutex>, + body: String, + } + + impl wiremock::Respond for GatedStsResponder { + fn respond(&self, _request: &wiremock::Request) -> ResponseTemplate { + let hit = self.hit.lock().unwrap().take(); + if let Some(hit) = hit { + let _ = hit.send(()); + } + let _ = self.release.lock().unwrap().recv(); + ResponseTemplate::new(200).set_body_string(self.body.clone()) + } + } + + #[tokio::test(flavor = "multi_thread", worker_threads = 4)] + async fn rotation_does_not_resurrect_refresh_deleted_mid_flight() { + let mock_server = MockServer::start().await; + let (hit_tx, hit_rx) = tokio::sync::oneshot::channel::<()>(); + let (release_tx, release_rx) = std::sync::mpsc::channel::<()>(); + Mock::given(method("POST")) + .and(body_string_contains("Action=AssumeRole")) + .respond_with(GatedStsResponder { + hit: std::sync::Mutex::new(Some(hit_tx)), + release: std::sync::Mutex::new(release_rx), + body: r#" + + + ASIAMOCKKEY + MockSecretAccessKey123 + MockSessionTokenXYZ + 2099-01-01T00:00:00Z + + +"# + .to_string(), + }) + .mount(&mock_server) + .await; + + let store = test_store().await; + crate::grpc::policy::set_global_bool_setting_for_test( + &store, + openshell_core::settings::PROVIDERS_V2_ENABLED_KEY, + true, + ) + .await + .unwrap(); + let prov = provider("aws-race", "aws"); + store.put_message(&prov).await.unwrap(); + let provider_id = prov.object_id().to_string(); + + let state = new_refresh_state( + &prov, + "AWS_ACCESS_KEY_ID", + NewRefreshStateConfig { + additional_output_keys: HashMap::from([ + ( + "secret_access_key".to_string(), + "AWS_SECRET_ACCESS_KEY".to_string(), + ), + ("session_token".to_string(), "AWS_SESSION_TOKEN".to_string()), + ]), + strategy: ProviderCredentialRefreshStrategy::AwsStsAssumeRole, + material: HashMap::from([ + ( + "role_arn".to_string(), + "arn:aws:iam::123456789012:role/TestRole".to_string(), + ), + ("aws_access_key_id".to_string(), "AKIATESTKEY".to_string()), + ( + "aws_secret_access_key".to_string(), + "TestSecretKey".to_string(), + ), + ("sts_endpoint_url".to_string(), mock_server.uri()), + ]), + secret_material_keys: vec!["aws_secret_access_key".to_string()], + expires_at_ms: 0, + token_url: String::new(), + scopes: Vec::new(), + refresh_before_seconds: 300, + max_lifetime_seconds: 3600, + }, + ) + .unwrap(); + put_refresh_state(&store, &state).await.unwrap(); + + let rotate = refresh_provider_credential(&store, "aws-race", "AWS_ACCESS_KEY_ID"); + let interfere = async { + // Wait until the rotation is inside the STS call (its state read has + // already happened), then delete the refresh and release STS. + if tokio::time::timeout(std::time::Duration::from_secs(15), hit_rx) + .await + .is_err() + { + return; + } + delete_refresh_state(&store, &provider_id, "AWS_ACCESS_KEY_ID") + .await + .unwrap(); + let _ = release_tx.send(()); + }; + let (rotate_result, ()) = tokio::join!(rotate, interfere); + + // The rotation must fail rather than complete against a deleted refresh. + assert!( + rotate_result.is_err(), + "rotation should abort when its refresh is deleted mid-flight" + ); + // The deleted refresh state must not be resurrected. + assert!( + get_refresh_state(&store, &provider_id, "AWS_ACCESS_KEY_ID") + .await + .unwrap() + .is_none(), + "deleted refresh state must not be recreated" + ); + // No credentials were minted into the provider. + let stored = store + .get_message_by_name::("aws-race") + .await + .unwrap() + .unwrap(); + assert!(!stored.credentials.contains_key("AWS_ACCESS_KEY_ID")); + assert!(!stored.credentials.contains_key("AWS_SECRET_ACCESS_KEY")); + assert!(!stored.credentials.contains_key("AWS_SESSION_TOKEN")); + } + fn provider(name: &str, provider_type: &str) -> Provider { Provider { metadata: Some(ObjectMeta { From bf68e0b29482ff983b162138ebe0c9a0a35403ac Mon Sep 17 00:00:00 2001 From: Russell Bryant Date: Tue, 14 Jul 2026 16:38:05 -0400 Subject: [PATCH 13/16] fix(providers): serialize refresh configuration to reserve keys atomically MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Refresh configuration validated key availability and persisted the reservation as separate steps. Two concurrent configures of providers attached to the same sandbox could each pass validation before either persisted, then both reserve the same primary or additional output key, leaving conflicting unusable refreshes (CWE-362). Hold the process-wide sandbox sync guard across the reserve-and-persist sequence — the same guard sandbox create/attach and profile changes take — so configurations and sandbox mutations that share the credential-key namespace are serialized. Add a concurrent-configure regression test asserting exactly one configuration reserves the key. Signed-off-by: Russell Bryant --- crates/openshell-server/src/grpc/provider.rs | 99 ++++++++++++++++++++ 1 file changed, 99 insertions(+) diff --git a/crates/openshell-server/src/grpc/provider.rs b/crates/openshell-server/src/grpc/provider.rs index 7f854415c9..6811d5a7b6 100644 --- a/crates/openshell-server/src/grpc/provider.rs +++ b/crates/openshell-server/src/grpc/provider.rs @@ -2158,6 +2158,14 @@ pub(super) async fn handle_configure_provider_refresh( )); } + // Serialize the reserve-then-persist sequence against other configurations + // and sandbox mutations. The collision validation below and the refresh-state + // persist further down are otherwise separate steps: two concurrent + // configures of providers attached to the same sandbox could each pass + // validation before either persisted and both reserve the same key (CWE-362). + // This is the same guard sandbox create/attach and profile changes take. + let _sandbox_sync_guard = state.compute.sandbox_sync_guard().await; + let provider = state .store .get_message_by_name::(provider_name) @@ -7085,6 +7093,97 @@ mod tests { assert!(err.message().contains("AWS_SECRET_ACCESS_KEY")); } + #[tokio::test(flavor = "multi_thread", worker_threads = 4)] + async fn concurrent_configure_aws_sts_reserves_key_exactly_once() { + use crate::grpc::policy::set_global_bool_setting_for_test; + + let state = test_server_state().await; + set_global_bool_setting_for_test( + state.store.as_ref(), + openshell_core::settings::PROVIDERS_V2_ENABLED_KEY, + true, + ) + .await + .unwrap(); + + for name in ["aws-a", "aws-b"] { + create_provider_record( + state.store.as_ref(), + Provider { + metadata: Some(openshell_core::proto::datamodel::v1::ObjectMeta { + id: String::new(), + name: name.to_string(), + created_at_ms: 0, + labels: HashMap::new(), + resource_version: 0, + }), + r#type: "aws".to_string(), + credentials: HashMap::new(), + config: HashMap::new(), + credential_expires_at_ms: HashMap::new(), + }, + ) + .await + .unwrap(); + } + + // Both providers are attached to the same sandbox, so their minted AWS + // credential env keys share a namespace and cannot both be reserved. + state + .store + .put_message(&Sandbox { + metadata: Some(openshell_core::proto::datamodel::v1::ObjectMeta { + id: "sandbox-concurrent-configure".to_string(), + name: "concurrent-configure".to_string(), + created_at_ms: 1, + labels: HashMap::new(), + resource_version: 0, + }), + spec: Some(SandboxSpec { + providers: vec!["aws-a".to_string(), "aws-b".to_string()], + ..SandboxSpec::default() + }), + ..Default::default() + }) + .await + .unwrap(); + + let configure = |provider: &str| { + Request::new(ConfigureProviderRefreshRequest { + provider: provider.to_string(), + credential_key: "AWS_ACCESS_KEY_ID".to_string(), + strategy: ProviderCredentialRefreshStrategy::AwsStsAssumeRole as i32, + material: HashMap::from([( + "role_arn".to_string(), + "arn:aws:iam::123456789012:role/Test".to_string(), + )]), + secret_material_keys: Vec::new(), + expires_at_ms: None, + }) + }; + + let (result_a, result_b) = tokio::join!( + handle_configure_provider_refresh(&state, configure("aws-a")), + handle_configure_provider_refresh(&state, configure("aws-b")), + ); + + // Exactly one configuration wins; the other is rejected as a collision + // rather than both reserving the same key. + let ok_count = [&result_a, &result_b].iter().filter(|r| r.is_ok()).count(); + assert_eq!( + ok_count, + 1, + "exactly one concurrent configure should reserve the key; got a={:?} b={:?}", + result_a.as_ref().map(|_| ()), + result_b.as_ref().map(|_| ()) + ); + let err = [result_a, result_b] + .into_iter() + .find_map(Result::err) + .expect("one configure must fail"); + assert_eq!(err.code(), Code::FailedPrecondition); + } + fn google_cloud_provider(config: HashMap) -> Provider { Provider { metadata: Some(openshell_core::proto::datamodel::v1::ObjectMeta { From 31c339ef331ae4bb8b1232af4e5eccdeea640d65 Mon Sep 17 00:00:00 2001 From: Russell Bryant Date: Tue, 14 Jul 2026 17:09:01 -0400 Subject: [PATCH 14/16] fix(persistence): return typed Conflict for absent-row conditional writes A version-matched put_if that matched no row reported an absent row as a stringy Database error while a version mismatch was a typed Conflict. Callers that need to distinguish 'condition not met' from a real error had to match backend-dependent error text. Report both cases as Conflict: an absent row carries current_resource_version: None, a version mismatch carries the current version. Applied to both the SQLite and Postgres backends. Signed-off-by: Russell Bryant --- .../src/persistence/postgres.rs | 19 +++++++++---------- .../src/persistence/sqlite.rs | 18 +++++++++--------- 2 files changed, 18 insertions(+), 19 deletions(-) diff --git a/crates/openshell-server/src/persistence/postgres.rs b/crates/openshell-server/src/persistence/postgres.rs index bb32a8efd6..968809e78d 100644 --- a/crates/openshell-server/src/persistence/postgres.rs +++ b/crates/openshell-server/src/persistence/postgres.rs @@ -160,17 +160,16 @@ RETURNING resource_version, created_at_ms, updated_at_ms updated_at_ms: row.get("updated_at_ms"), }) } else { - // Check if object exists to distinguish NotFound from Conflict + // The version-matched UPDATE matched no row. Distinguish a + // version mismatch (row present, different version) from an + // absent row (deleted / never existed). Both are CAS + // precondition failures, so report them as typed `Conflict` + // rather than a backend-dependent error string: absent rows + // carry `current_resource_version: None`. let existing = self.get(object_type, id).await?; - if let Some(record) = existing { - Err(PersistenceError::Conflict { - current_resource_version: Some(record.resource_version), - }) - } else { - Err(PersistenceError::Database(format!( - "object not found: {object_type}/{id}" - ))) - } + Err(PersistenceError::Conflict { + current_resource_version: existing.map(|record| record.resource_version), + }) } } WriteCondition::Unconditional => { diff --git a/crates/openshell-server/src/persistence/sqlite.rs b/crates/openshell-server/src/persistence/sqlite.rs index efb6a9bf45..dd6996bea8 100644 --- a/crates/openshell-server/src/persistence/sqlite.rs +++ b/crates/openshell-server/src/persistence/sqlite.rs @@ -168,16 +168,16 @@ WHERE "object_type" = ?1 AND "id" = ?2 AND "resource_version" = ?3 .map_err(|e| map_db_error(&e))?; if result.rows_affected() == 0 { - // Check if object exists to distinguish NotFound from Conflict + // The version-matched UPDATE matched no row. Distinguish a + // version mismatch (row present, different version) from an + // absent row (deleted / never existed). Both are CAS + // precondition failures, so report them as typed `Conflict` + // rather than a backend-dependent error string: absent rows + // carry `current_resource_version: None`. let existing = self.get(object_type, id).await?; - if let Some(record) = existing { - return Err(PersistenceError::Conflict { - current_resource_version: Some(record.resource_version), - }); - } - return Err(PersistenceError::Database(format!( - "object not found: {object_type}/{id}" - ))); + return Err(PersistenceError::Conflict { + current_resource_version: existing.map(|record| record.resource_version), + }); } // Fetch the updated record to get the new resource_version From a5a89f036b1441f8d228766a40890086f2ea1488 Mon Sep 17 00:00:00 2001 From: Russell Bryant Date: Tue, 14 Jul 2026 17:09:13 -0400 Subject: [PATCH 15/16] fix(providers): gate provider credential write on refresh generation ownership MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit After minting, the rotation checked only that a refresh with the same name still existed, wrote credentials to the provider, and only then did the version-matched refresh-state write. A refresh deleted-and-recreated during mint, or a losing concurrent rotation, could write credentials minted from a stale generation before returning Aborted (CWE-362). Claim the refresh generation first: perform the version-matched refresh-state write before touching the provider. It succeeds only if the refresh still holds the generation the rotation started from; on a lost claim (deleted, recreated, or superseded) it returns None and the rotation aborts leaving both the refresh state and provider unchanged. No transaction spans the two objects — the store exposes only per-object CAS — so this optimistic claim-then-write is the ownership guarantee: only the generation owner writes credentials. Uses the typed conditional-write Conflict result instead of matching error strings. Adds a deterministic superseded-mid-flight regression test (a concurrent write bumps the generation while STS is paused) alongside the existing deleted-mid-flight test. Signed-off-by: Russell Bryant --- .../openshell-server/src/provider_refresh.rs | 219 +++++++++++++----- 1 file changed, 166 insertions(+), 53 deletions(-) diff --git a/crates/openshell-server/src/provider_refresh.rs b/crates/openshell-server/src/provider_refresh.rs index aa6a98ff54..36804e6e62 100644 --- a/crates/openshell-server/src/provider_refresh.rs +++ b/crates/openshell-server/src/provider_refresh.rs @@ -52,14 +52,14 @@ pub async fn put_refresh_state( /// /// Uses a version-matched UPDATE, which never inserts — so a refresh deleted /// while an STS (or OAuth) request was in flight is not resurrected, and its -/// stored source-credential material is not recreated (CWE-362). Returns `true` -/// when persisted, `false` when the refresh was deleted or superseded by a -/// concurrent write (in which case nothing was written). +/// stored source-credential material is not recreated (CWE-362). Returns the new +/// resource version when persisted, or `None` when the refresh was deleted or +/// superseded by a concurrent write (in which case nothing was written). async fn persist_refresh_state_if_current( store: &Store, state: &StoredProviderCredentialRefreshState, expected_version: u64, -) -> Result { +) -> Result, Status> { match store .put_if( StoredProviderCredentialRefreshState::object_type(), @@ -71,11 +71,11 @@ async fn persist_refresh_state_if_current( ) .await { - Ok(_) => Ok(true), - // Version mismatch (superseded) or the row is gone (deleted): the - // version-matched UPDATE matched no row, so nothing was written. - Err(PersistenceError::Conflict { .. }) => Ok(false), - Err(PersistenceError::Database(message)) if message.contains("not found") => Ok(false), + Ok(result) => Ok(Some(result.resource_version)), + // The version-matched UPDATE matched no row: the refresh was deleted + // (current version `None`) or superseded by a concurrent write (a + // different current version). Either way nothing was written. + Err(PersistenceError::Conflict { .. }) => Ok(None), Err(e) => Err(Status::internal(format!( "persist provider refresh state failed: {e}" ))), @@ -383,47 +383,9 @@ pub async fn refresh_provider_credential( match mint_credential(&state).await { Ok(minted) => { let now_ms = current_time_ms(); - // A delete-refresh can complete while the mint request is in flight. - // Verify the refresh still exists before writing minted credentials - // into the provider, so a removed refresh is not silently re-minted - // (CWE-362). The version-matched persist below is the authoritative - // guard against resurrecting deleted state; this check just avoids - // writing credentials we are about to discard. - if get_refresh_state(store, provider.object_id(), credential_key) - .await? - .is_none() - { - warn!( - provider = %state.provider_name, - credential_key = %state.credential_key, - strategy = %refresh_strategy_name(state.strategy), - "provider credential refresh deleted during rotation; discarding minted credentials" - ); - return Err(Status::aborted( - "provider refresh was deleted during rotation", - )); - } - if let Err(err) = - apply_minted_credential(store, &provider, credential_key, &minted).await - { - state.status = "error".to_string(); - state.last_error = err.message().to_string(); - state.next_refresh_at_ms = - now_ms.saturating_add(REFRESH_ERROR_RETRY_SECONDS.saturating_mul(1000)); - persist_refresh_state_if_current(store, &state, expected_version).await?; - warn!( - provider = %state.provider_name, - credential_key = %state.credential_key, - strategy = %refresh_strategy_name(state.strategy), - status = %state.status, - next_refresh_at_ms = state.next_refresh_at_ms, - seconds_until_refresh = seconds_until_ms(now_ms, state.next_refresh_at_ms), - error = %err, - "provider credential refresh errored" - ); - return Err(err); - } - if let Some(refresh_token) = minted.refresh_token { + // Fold the minted result into the refresh state before claiming the + // generation. + if let Some(refresh_token) = minted.refresh_token.clone() { state .material .insert("refresh_token".to_string(), refresh_token); @@ -445,16 +407,51 @@ pub async fn refresh_provider_credential( state.last_refresh_at_ms = now_ms; state.status = "refreshed".to_string(); state.last_error.clear(); - if !persist_refresh_state_if_current(store, &state, expected_version).await? { + + // Claim the refresh generation with a version-matched write BEFORE + // touching the provider. It succeeds only if the refresh still holds + // the generation we started from; if it was deleted, recreated, or a + // concurrent rotation won, it returns `None` and we leave both the + // refresh state and the provider unchanged — no credentials minted + // from a stale generation are written, and a deleted refresh is not + // resurrected (CWE-362). This makes generation ownership the gate on + // the provider credential write. + let Some(new_version) = + persist_refresh_state_if_current(store, &state, expected_version).await? + else { warn!( provider = %state.provider_name, credential_key = %state.credential_key, strategy = %refresh_strategy_name(state.strategy), - "provider credential refresh deleted or superseded during rotation; not resurrecting refresh state" + "provider credential refresh deleted or superseded during rotation; discarding minted credentials" ); return Err(Status::aborted( - "provider refresh was deleted during rotation", + "provider refresh was deleted or superseded during rotation", )); + }; + + // Generation is ours; write the minted credentials into the provider. + if let Err(err) = + apply_minted_credential(store, &provider, credential_key, &minted).await + { + state.status = "error".to_string(); + state.last_error = err.message().to_string(); + state.next_refresh_at_ms = + now_ms.saturating_add(REFRESH_ERROR_RETRY_SECONDS.saturating_mul(1000)); + // Reflect the failure on the state we just wrote; skip silently + // if it was deleted concurrently (it is not recreated). + persist_refresh_state_if_current(store, &state, new_version).await?; + warn!( + provider = %state.provider_name, + credential_key = %state.credential_key, + strategy = %refresh_strategy_name(state.strategy), + status = %state.status, + next_refresh_at_ms = state.next_refresh_at_ms, + seconds_until_refresh = seconds_until_ms(now_ms, state.next_refresh_at_ms), + error = %err, + "provider credential refresh errored" + ); + return Err(err); } info!( provider = %state.provider_name, @@ -1954,6 +1951,122 @@ mod tests { assert!(!stored.credentials.contains_key("AWS_SESSION_TOKEN")); } + #[tokio::test(flavor = "multi_thread", worker_threads = 4)] + async fn rotation_superseded_mid_flight_discards_credentials() { + let mock_server = MockServer::start().await; + let (hit_tx, hit_rx) = tokio::sync::oneshot::channel::<()>(); + let (release_tx, release_rx) = std::sync::mpsc::channel::<()>(); + Mock::given(method("POST")) + .and(body_string_contains("Action=AssumeRole")) + .respond_with(GatedStsResponder { + hit: std::sync::Mutex::new(Some(hit_tx)), + release: std::sync::Mutex::new(release_rx), + body: r#" + + + ASIAMOCKKEY + MockSecretAccessKey123 + MockSessionTokenXYZ + 2099-01-01T00:00:00Z + + +"# + .to_string(), + }) + .mount(&mock_server) + .await; + + let store = test_store().await; + crate::grpc::policy::set_global_bool_setting_for_test( + &store, + openshell_core::settings::PROVIDERS_V2_ENABLED_KEY, + true, + ) + .await + .unwrap(); + let prov = provider("aws-superseded", "aws"); + store.put_message(&prov).await.unwrap(); + let provider_id = prov.object_id().to_string(); + + let state = new_refresh_state( + &prov, + "AWS_ACCESS_KEY_ID", + NewRefreshStateConfig { + additional_output_keys: HashMap::from([ + ( + "secret_access_key".to_string(), + "AWS_SECRET_ACCESS_KEY".to_string(), + ), + ("session_token".to_string(), "AWS_SESSION_TOKEN".to_string()), + ]), + strategy: ProviderCredentialRefreshStrategy::AwsStsAssumeRole, + material: HashMap::from([ + ( + "role_arn".to_string(), + "arn:aws:iam::123456789012:role/TestRole".to_string(), + ), + ("aws_access_key_id".to_string(), "AKIATESTKEY".to_string()), + ( + "aws_secret_access_key".to_string(), + "TestSecretKey".to_string(), + ), + ("sts_endpoint_url".to_string(), mock_server.uri()), + ]), + secret_material_keys: vec!["aws_secret_access_key".to_string()], + expires_at_ms: 0, + token_url: String::new(), + scopes: Vec::new(), + refresh_before_seconds: 300, + max_lifetime_seconds: 3600, + }, + ) + .unwrap(); + put_refresh_state(&store, &state).await.unwrap(); + + let rotate = refresh_provider_credential(&store, "aws-superseded", "AWS_ACCESS_KEY_ID"); + let interfere = async { + if tokio::time::timeout(std::time::Duration::from_secs(15), hit_rx) + .await + .is_err() + { + return; + } + // Simulate a concurrent rotation or reconfigure winning the + // generation: any write to the refresh state bumps its version, so + // the in-flight rotation's version-matched persist will lose. + let mut winner = get_refresh_state(&store, &provider_id, "AWS_ACCESS_KEY_ID") + .await + .unwrap() + .unwrap(); + winner.last_error = "won-by-concurrent-writer".to_string(); + put_refresh_state(&store, &winner).await.unwrap(); + let _ = release_tx.send(()); + }; + let (rotate_result, ()) = tokio::join!(rotate, interfere); + + // The superseded (losing) rotation must abort rather than complete. + assert!( + rotate_result.is_err(), + "a rotation whose generation was superseded must abort" + ); + // It must not write its stale-generation credentials into the provider. + let stored = store + .get_message_by_name::("aws-superseded") + .await + .unwrap() + .unwrap(); + assert!(!stored.credentials.contains_key("AWS_ACCESS_KEY_ID")); + assert!(!stored.credentials.contains_key("AWS_SECRET_ACCESS_KEY")); + assert!(!stored.credentials.contains_key("AWS_SESSION_TOKEN")); + // The concurrent writer's refresh state must survive untouched. + let refresh = get_refresh_state(&store, &provider_id, "AWS_ACCESS_KEY_ID") + .await + .unwrap() + .expect("refresh state should still exist"); + assert_eq!(refresh.last_error, "won-by-concurrent-writer"); + assert_ne!(refresh.status, "refreshed"); + } + fn provider(name: &str, provider_type: &str) -> Provider { Provider { metadata: Some(ObjectMeta { From b0365a117f3872c17d7b9f967c1977b7d1d63981 Mon Sep 17 00:00:00 2001 From: Russell Bryant Date: Tue, 14 Jul 2026 17:13:33 -0400 Subject: [PATCH 16/16] feat(providers): support aws_session_token for temporary source credentials The explicit AWS source-credential path always passed no session token, so temporary credentials from AWS SSO or a prior AssumeRole (which are only valid with a session token) failed to sign the STS request. Accept an optional secret aws_session_token material and pass it to the AWS SDK. It requires the aws_access_key_id and aws_secret_access_key pair, enforced at both the configure boundary and the mint path. Declare it as optional secret material on the aws and aws-s3 profiles and document it. Signed-off-by: Russell Bryant --- crates/openshell-server/src/grpc/provider.rs | 70 ++++++++ .../openshell-server/src/provider_refresh.rs | 152 +++++++++++++++++- docs/sandboxes/manage-providers.mdx | 4 + providers/aws-s3.yaml | 4 + providers/aws.yaml | 4 + 5 files changed, 233 insertions(+), 1 deletion(-) diff --git a/crates/openshell-server/src/grpc/provider.rs b/crates/openshell-server/src/grpc/provider.rs index 6811d5a7b6..76aae31177 100644 --- a/crates/openshell-server/src/grpc/provider.rs +++ b/crates/openshell-server/src/grpc/provider.rs @@ -2149,6 +2149,17 @@ pub(super) async fn handle_configure_provider_refresh( "aws_access_key_id and aws_secret_access_key must both be set or both omitted", )); } + // An optional session token supports temporary source credentials (SSO or a + // prior AssumeRole). It only makes sense alongside the source key pair. + let has_source_session_token = request + .material + .get("aws_session_token") + .is_some_and(|value| !value.trim().is_empty()); + if has_source_session_token && !has_source_access_key { + return Err(Status::invalid_argument( + "aws_session_token requires aws_access_key_id and aws_secret_access_key", + )); + } if request .expires_at_ms .is_some_and(|expires_at_ms| expires_at_ms < 0) @@ -6622,6 +6633,65 @@ mod tests { assert!(err.message().contains("both be set or both omitted")); } + #[tokio::test] + async fn configure_aws_sts_rejects_session_token_without_pair() { + use crate::grpc::policy::set_global_bool_setting_for_test; + + let state = test_server_state().await; + set_global_bool_setting_for_test( + state.store.as_ref(), + openshell_core::settings::PROVIDERS_V2_ENABLED_KEY, + true, + ) + .await + .unwrap(); + + create_provider_record( + state.store.as_ref(), + Provider { + metadata: Some(openshell_core::proto::datamodel::v1::ObjectMeta { + id: String::new(), + name: "aws-lone-session".to_string(), + created_at_ms: 0, + labels: HashMap::new(), + resource_version: 0, + }), + r#type: "aws".to_string(), + credentials: HashMap::new(), + config: HashMap::new(), + credential_expires_at_ms: HashMap::new(), + }, + ) + .await + .unwrap(); + + let err = handle_configure_provider_refresh( + &state, + Request::new(ConfigureProviderRefreshRequest { + provider: "aws-lone-session".to_string(), + credential_key: "AWS_ACCESS_KEY_ID".to_string(), + strategy: ProviderCredentialRefreshStrategy::AwsStsAssumeRole as i32, + material: HashMap::from([ + ( + "role_arn".to_string(), + "arn:aws:iam::123456789012:role/Test".to_string(), + ), + ( + "aws_session_token".to_string(), + "temp-session-token".to_string(), + ), + ]), + secret_material_keys: vec!["aws_session_token".to_string()], + expires_at_ms: None, + }), + ) + .await + .unwrap_err(); + + assert_eq!(err.code(), Code::InvalidArgument); + assert!(err.message().contains("aws_session_token requires")); + } + #[tokio::test] async fn configure_aws_sts_persists_resolved_additional_output_keys() { use crate::grpc::StoredSettingValue; diff --git a/crates/openshell-server/src/provider_refresh.rs b/crates/openshell-server/src/provider_refresh.rs index 36804e6e62..7843eadf8c 100644 --- a/crates/openshell-server/src/provider_refresh.rs +++ b/crates/openshell-server/src/provider_refresh.rs @@ -701,6 +701,9 @@ async fn mint_aws_sts_assume_role( // Explicit source credentials are all-or-nothing. A lone key must not // silently fall through to the gateway's ambient identity (CWE-20): the // caller asked for a specific principal, so an incomplete pair is an error. + // An optional session token supports temporary source credentials (SSO or a + // prior AssumeRole); it requires the access/secret pair. + let session_token = material_value(&state.material, &["aws_session_token"]); match ( material_value(&state.material, &["aws_access_key_id"]), material_value(&state.material, &["aws_secret_access_key"]), @@ -709,12 +712,17 @@ async fn mint_aws_sts_assume_role( let creds = aws_sdk_sts::config::Credentials::new( access_key, secret_key, - None, + session_token, None, "openshell-provider-refresh", ); config_loader = config_loader.credentials_provider(creds); } + (None, None) if session_token.is_some() => { + return Err(Status::invalid_argument( + "aws_session_token requires aws_access_key_id and aws_secret_access_key", + )); + } (None, None) => {} _ => { return Err(Status::invalid_argument( @@ -1838,6 +1846,148 @@ mod tests { } } + #[tokio::test] + async fn aws_sts_mint_accepts_session_token_with_source_pair() { + let mock_server = MockServer::start().await; + Mock::given(method("POST")) + .and(body_string_contains("Action=AssumeRole")) + .respond_with(ResponseTemplate::new(200).set_body_string( + r#" + + + ASIAMOCKKEY + MockSecretAccessKey123 + MockSessionTokenXYZ + 2099-01-01T00:00:00Z + + +"#, + )) + .mount(&mock_server) + .await; + + let store = test_store().await; + crate::grpc::policy::set_global_bool_setting_for_test( + &store, + openshell_core::settings::PROVIDERS_V2_ENABLED_KEY, + true, + ) + .await + .unwrap(); + let prov = provider("aws-sts-session", "aws"); + store.put_message(&prov).await.unwrap(); + + // Temporary source credentials: access key + secret + session token. + let state = new_refresh_state( + &prov, + "AWS_ACCESS_KEY_ID", + NewRefreshStateConfig { + additional_output_keys: HashMap::from([ + ( + "secret_access_key".to_string(), + "AWS_SECRET_ACCESS_KEY".to_string(), + ), + ("session_token".to_string(), "AWS_SESSION_TOKEN".to_string()), + ]), + strategy: ProviderCredentialRefreshStrategy::AwsStsAssumeRole, + material: HashMap::from([ + ( + "role_arn".to_string(), + "arn:aws:iam::123456789012:role/TestRole".to_string(), + ), + ("aws_access_key_id".to_string(), "ASIASOURCEKEY".to_string()), + ( + "aws_secret_access_key".to_string(), + "SourceSecretKey".to_string(), + ), + ( + "aws_session_token".to_string(), + "SourceSessionToken".to_string(), + ), + ("sts_endpoint_url".to_string(), mock_server.uri()), + ]), + secret_material_keys: vec![ + "aws_secret_access_key".to_string(), + "aws_session_token".to_string(), + ], + expires_at_ms: 0, + token_url: String::new(), + scopes: Vec::new(), + refresh_before_seconds: 300, + max_lifetime_seconds: 3600, + }, + ) + .unwrap(); + put_refresh_state(&store, &state).await.unwrap(); + + let refreshed = refresh_provider_credential(&store, "aws-sts-session", "AWS_ACCESS_KEY_ID") + .await + .unwrap(); + assert_eq!(refreshed.status, "refreshed"); + let stored = store + .get_message_by_name::("aws-sts-session") + .await + .unwrap() + .unwrap(); + assert_eq!( + stored.credentials.get("AWS_ACCESS_KEY_ID"), + Some(&"ASIAMOCKKEY".to_string()) + ); + } + + #[tokio::test] + async fn aws_sts_mint_rejects_session_token_without_source_pair() { + let store = test_store().await; + crate::grpc::policy::set_global_bool_setting_for_test( + &store, + openshell_core::settings::PROVIDERS_V2_ENABLED_KEY, + true, + ) + .await + .unwrap(); + let prov = provider("aws-sts-lonesession", "aws"); + store.put_message(&prov).await.unwrap(); + + let state = new_refresh_state( + &prov, + "AWS_ACCESS_KEY_ID", + NewRefreshStateConfig { + additional_output_keys: HashMap::from([ + ( + "secret_access_key".to_string(), + "AWS_SECRET_ACCESS_KEY".to_string(), + ), + ("session_token".to_string(), "AWS_SESSION_TOKEN".to_string()), + ]), + strategy: ProviderCredentialRefreshStrategy::AwsStsAssumeRole, + material: HashMap::from([ + ( + "role_arn".to_string(), + "arn:aws:iam::123456789012:role/TestRole".to_string(), + ), + ( + "aws_session_token".to_string(), + "SourceSessionToken".to_string(), + ), + ]), + secret_material_keys: vec!["aws_session_token".to_string()], + expires_at_ms: 0, + token_url: String::new(), + scopes: Vec::new(), + refresh_before_seconds: 300, + max_lifetime_seconds: 3600, + }, + ) + .unwrap(); + put_refresh_state(&store, &state).await.unwrap(); + + let err = refresh_provider_credential(&store, "aws-sts-lonesession", "AWS_ACCESS_KEY_ID") + .await + .unwrap_err(); + assert_eq!(err.code(), tonic::Code::InvalidArgument); + assert!(err.message().contains("aws_session_token requires")); + } + #[tokio::test(flavor = "multi_thread", worker_threads = 4)] async fn rotation_does_not_resurrect_refresh_deleted_mid_flight() { let mock_server = MockServer::start().await; diff --git a/docs/sandboxes/manage-providers.mdx b/docs/sandboxes/manage-providers.mdx index 2ea77dd1f3..d639c37acb 100644 --- a/docs/sandboxes/manage-providers.mdx +++ b/docs/sandboxes/manage-providers.mdx @@ -218,6 +218,10 @@ Pass the long-lived secret with `--secret-material-env` so the CLI reads it from its own environment instead of expanding it into a process argument, where it would be visible in the host process table. +For temporary source credentials (AWS SSO or a prior `AssumeRole`), also pass a +session token with `--secret-material-env aws_session_token=AWS_SESSION_TOKEN`. +It requires the `aws_access_key_id` and `aws_secret_access_key` pair. + Use the generic `aws` profile type for multi-service access and scope endpoints via sandbox network policy. Use `aws-s3` for S3-specific endpoint rules. diff --git a/providers/aws-s3.yaml b/providers/aws-s3.yaml index e8885924ee..699f067ff1 100644 --- a/providers/aws-s3.yaml +++ b/providers/aws-s3.yaml @@ -47,6 +47,10 @@ credentials: description: Long-lived IAM secret key (only needed if gateway lacks ambient AWS credentials) required: false secret: true + - name: aws_session_token + description: Session token for temporary source credentials (SSO or a prior AssumeRole); requires the access/secret pair + required: false + secret: true - name: secret_access_key description: AWS secret access key (co-managed with access_key_id) env_vars: [AWS_SECRET_ACCESS_KEY] diff --git a/providers/aws.yaml b/providers/aws.yaml index d63795ff58..90f13765f0 100644 --- a/providers/aws.yaml +++ b/providers/aws.yaml @@ -47,6 +47,10 @@ credentials: description: Long-lived IAM secret key (only needed if gateway lacks ambient AWS credentials) required: false secret: true + - name: aws_session_token + description: Session token for temporary source credentials (SSO or a prior AssumeRole); requires the access/secret pair + required: false + secret: true - name: secret_access_key description: AWS secret access key (co-managed with access_key_id) env_vars: [AWS_SECRET_ACCESS_KEY]