From 06f6899de262308f68e0e2df8048416acd3cce26 Mon Sep 17 00:00:00 2001 From: Roberto Catalano Date: Sun, 23 Aug 2026 22:13:56 +0200 Subject: [PATCH] fix: 3LO secrets by name, and provider config that actually renders MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The Google/GitHub/Notion credential providers had two defects, one hiding the other: 1. Client secrets travelled as plaintext (-c google_client_secret / env) and were rendered verbatim into the synthesized template — the exposure class the IdP secret fix already closed for module 4's federation path. 2. Worse, found while fixing (1): the provider config was passed as a raw dict whose top-level key spelled OAuth with a capital A, which does not match the CloudFormation model — the L1 mapping silently dropped the ENTIRE block. Every 3LO provider synthesized with Oauth2ProviderConfigInput: {}. They could never have worked. Same hazard class as the web-search connector (project memory has the earlier instance). Fixes, all following existing precedent in this repo: - identity_stack takes *_client_secret_name and renders {{resolve:secretsmanager:...}} dynamic references (auth-stack pattern); a client_id without a secret name fails at synth with the same actionable message shape as the auth stack. - Typed L1 property classes replace the raw dicts, so a wrong key raises at synth instead of vanishing. Google/GitHub use their vendor configs (the Token Vault knows their endpoints); Notion publishes no OIDC discovery, so it is CustomOauth2 with explicit authorization/token endpoints — the same shape as the deployed-and-working gateway M2M provider. The scopes lists were dead weight in the dropped dicts: scopes are requested per token via @requires_access_token, not stored on the provider. - app.py rejects the plaintext keys outright and reads names through cfg() (the test_platform_config guard now allows only the rejection loop, which deliberately probes keys that must NOT resolve). - deploy.sh: upsert_oauth_secret generalizes upsert_idp_secret (trim, bring-your-own name, plaintext unset, exported so context-arg-less CDK calls like the bootstrap probe see it — that export was found live when the first deploy failed only at the bootstrap step). upsert_3lo_secrets covers the three vendors; names ride as context args. Verified live on the test rig (deploy --stack agentcore-workshop-dev-identity with GITHUB_CLIENT_ID + a secret carrying a trailing newline): - provider created as GithubOauth2 with real vendor config (was {} before) - Secrets Manager value is the trimmed 20 chars, newline gone - the DEPLOYED CloudFormation template contains the dynamic reference and no plaintext (checked via get-template) - CfnOAuth2CredentialProvider accepts the dynamic reference (the task's open question) — UPDATE_COMPLETE both ways Rig restored: provider removed, test secret deleted, invoke.py green. Checks: check (m) in check-deploy-config.sh (trim + naming + plaintext unset across all three vendors); tests/test_3lo_providers.py guards the source shape (no plaintext params, dynamic refs, no raw-dict key, deploy.sh passes names only). 120 tests green, ruff clean, shellcheck clean. --- app.py | 47 +++++++------ scripts/check-deploy-config.sh | 30 ++++++++- scripts/deploy.sh | 89 ++++++++++++++++++------- stacks/identity_stack.py | 116 ++++++++++++++++++++++----------- tests/test_3lo_providers.py | 65 ++++++++++++++++++ tests/test_platform_config.py | 8 +-- 6 files changed, 268 insertions(+), 87 deletions(-) create mode 100644 tests/test_3lo_providers.py diff --git a/app.py b/app.py index 1652256..52eba87 100755 --- a/app.py +++ b/app.py @@ -157,25 +157,34 @@ def cfg(context_key: str, env_key: str, default: str) -> str: "issuer_url": cfg("idp_issuer_url", "IDP_ISSUER_URL", ""), } -# OAuth provider credentials -google_client_id = app.node.try_get_context("google_client_id") or os.environ.get( - "GOOGLE_CLIENT_ID", "" +# OAuth provider credentials (3LO). Secrets travel as Secrets Manager secret +# NAMES only — a plaintext *_client_secret context key or env var is rejected, +# because context values land in `ps` output, cdk.context.json, and (previously) +# verbatim in the synthesized template. deploy.sh upserts the secret and passes +# the name; see the identity stack for the dynamic-reference rendering. +for _vendor in ("google", "github", "notion"): + if app.node.try_get_context(f"{_vendor}_client_secret") or os.environ.get( + f"{_vendor.upper()}_CLIENT_SECRET", "" + ): + raise ValueError( + f"Plaintext '{_vendor}_client_secret' / {_vendor.upper()}_CLIENT_SECRET is " + f"no longer supported — store it in Secrets Manager and pass " + f"'{_vendor}_client_secret_name' instead (scripts/deploy.sh does this " + "automatically when the secret is in the environment)." + ) + +google_client_id = cfg("google_client_id", "GOOGLE_CLIENT_ID", "") +google_client_secret_name = cfg( + "google_client_secret_name", "GOOGLE_CLIENT_SECRET_NAME", "" ) -google_client_secret = app.node.try_get_context( - "google_client_secret" -) or os.environ.get("GOOGLE_CLIENT_SECRET", "") -github_client_id = app.node.try_get_context("github_client_id") or os.environ.get( - "GITHUB_CLIENT_ID", "" +github_client_id = cfg("github_client_id", "GITHUB_CLIENT_ID", "") +github_client_secret_name = cfg( + "github_client_secret_name", "GITHUB_CLIENT_SECRET_NAME", "" ) -github_client_secret = app.node.try_get_context( - "github_client_secret" -) or os.environ.get("GITHUB_CLIENT_SECRET", "") -notion_client_id = app.node.try_get_context("notion_client_id") or os.environ.get( - "NOTION_CLIENT_ID", "" +notion_client_id = cfg("notion_client_id", "NOTION_CLIENT_ID", "") +notion_client_secret_name = cfg( + "notion_client_secret_name", "NOTION_CLIENT_SECRET_NAME", "" ) -notion_client_secret = app.node.try_get_context( - "notion_client_secret" -) or os.environ.get("NOTION_CLIENT_SECRET", "") # ── Global Tags ── cdk.Tags.of(app).add("Project", project) @@ -279,11 +288,11 @@ def cfg(context_key: str, env_key: str, default: str) -> str: gateway_m2m_client_secret=m2m_client_secret, cognito_discovery_url=discovery_url, google_client_id=google_client_id, - google_client_secret=google_client_secret, + google_client_secret_name=google_client_secret_name, github_client_id=github_client_id, - github_client_secret=github_client_secret, + github_client_secret_name=github_client_secret_name, notion_client_id=notion_client_id, - notion_client_secret=notion_client_secret, + notion_client_secret_name=notion_client_secret_name, env=cdk_env, ) if auth_stack: diff --git a/scripts/check-deploy-config.sh b/scripts/check-deploy-config.sh index 2b2bc46..05b89e5 100755 --- a/scripts/check-deploy-config.sh +++ b/scripts/check-deploy-config.sh @@ -173,7 +173,7 @@ rm -f "$PLATFORM_CONFIG" # A trailing newline (pasted, or piped from `az ... -o tsv`) is stored verbatim, # Cognito forwards it to the IdP token endpoint, and the exchange fails with # invalid_client mentioning nothing about whitespace. -eval "$(sed -n '/^upsert_idp_secret()/,/^}/p' "$SCRIPT_DIR/deploy.sh")" +eval "$(sed -n '/^upsert_oauth_secret()/,/^}/p; /^upsert_idp_secret()/,/^}/p; /^upsert_3lo_secrets()/,/^}/p' "$SCRIPT_DIR/deploy.sh")" # The function unsets the plaintext when it is done (deliberate hygiene), so # assert on what it PASSED to the CLI rather than on the variable afterwards. # SC2034/SC2329: PREFIX, AWS_REGION and IDP_CLIENT_SECRET are read by the @@ -242,4 +242,32 @@ grep -q -- "put-secret-value --secret-id my-corp/entra-secret" "$TMP/aws.args" \ unset -f aws prompt_idp echo "PASS: a configured IdP secret name is reused, not duplicated" +# (m) 3LO client secrets follow the same road: trimmed, stored under the +# prefixed name (or a configured one), plaintext unset afterwards. These used +# to be rendered verbatim into the synthesized template via cdk context. +# shellcheck disable=SC2329 +aws() { printf '%s\n' "$*" >> "$TMP/aws.args"; return 0; } +: > "$TMP/aws.args" +# shellcheck disable=SC2034 # read via indirection in the eval'd functions +GOOGLE_CLIENT_SECRET=$'g-sekret\n' +# shellcheck disable=SC2034 +GITHUB_CLIENT_SECRET=" gh-sekret " +# shellcheck disable=SC2034 +NOTION_CLIENT_SECRET_NAME="my-corp/notion" # bring-your-own name +# shellcheck disable=SC2034 +NOTION_CLIENT_SECRET="n-sekret" +upsert_3lo_secrets >/dev/null 2>&1 || true +grep -q -- "--secret-string g-sekret " "$TMP/aws.args" \ + || fail "google secret newline not stripped: $(cat "$TMP/aws.args")" +grep -q -- "--secret-string gh-sekret " "$TMP/aws.args" \ + || fail "github secret padding not stripped: $(cat "$TMP/aws.args")" +grep -q -- "--secret-id my-corp/notion" "$TMP/aws.args" \ + || fail "notion bring-your-own name ignored: $(cat "$TMP/aws.args")" +[ "$GOOGLE_CLIENT_SECRET_NAME" = "check-prefix-google-oauth-secret" ] \ + || fail "google secret name not defaulted: ${GOOGLE_CLIENT_SECRET_NAME:-unset}" +[ -z "${GOOGLE_CLIENT_SECRET:-}${GITHUB_CLIENT_SECRET:-}${NOTION_CLIENT_SECRET:-}" ] \ + || fail "a 3LO plaintext survived the upsert" +unset -f aws +echo "PASS: 3LO client secrets are trimmed, named, and never persisted" + echo "OK: all deploy-config checks passed" diff --git a/scripts/deploy.sh b/scripts/deploy.sh index 41d7845..587a315 100755 --- a/scripts/deploy.sh +++ b/scripts/deploy.sh @@ -468,43 +468,72 @@ prompt_api_keys() { # template). Instead we upsert it into Secrets Manager and pass only the # secret NAME; the auth stack resolves the value at deploy time via a # {{resolve:secretsmanager:...}} CloudFormation dynamic reference. -upsert_idp_secret() { - if [ -z "${IDP_CLIENT_SECRET:-}" ]; then return 0; fi +# +# upsert_oauth_secret VALUE_VAR NAME_VAR DEFAULT_NAME LABEL +# VALUE_VAR name of the variable holding the plaintext (unset afterwards) +# NAME_VAR name of the variable holding/receiving the secret's name +# DEFAULT_NAME secret name used when the operator did not configure one +# LABEL human label for log messages +upsert_oauth_secret() { + local value_var="$1" name_var="$2" default_name="$3" label="$4" + local value="${!value_var:-}" + if [ -z "$value" ]; then return 0; fi # Strip surrounding whitespace. A secret pasted from a console, or piped in - # from `az ad app credential reset -o tsv`, arrives with a trailing newline; - # Cognito forwards it verbatim to the IdP's token endpoint and the exchange - # fails with invalid_client, naming nothing about whitespace. Cost an hour - # to find live — see docs/ENTERPRISE_IDP.md. - IDP_CLIENT_SECRET="$(printf '%s' "$IDP_CLIENT_SECRET" | tr -d '\n\r' | sed -e 's/^[[:space:]]*//' -e 's/[[:space:]]*$//')" - if [ -z "$IDP_CLIENT_SECRET" ]; then - log_error "IDP_CLIENT_SECRET is only whitespace — nothing to store." + # from a CLI (`az ... -o tsv`), arrives with a trailing newline; it is + # stored and forwarded verbatim, and the provider's token endpoint rejects + # the exchange with invalid_client, naming nothing about whitespace. Cost + # an hour to find live — see docs/ENTERPRISE_IDP.md. + value="$(printf '%s' "$value" | tr -d '\n\r' | sed -e 's/^[[:space:]]*//' -e 's/[[:space:]]*$//')" + if [ -z "$value" ]; then + log_error "$value_var is only whitespace — nothing to store." exit 1 fi # Write to the operator's secret when they named one (platform.yaml / env), # otherwise to our own. Rotating into a bring-your-own secret must not # silently fork a second copy under the prefixed name. - IDP_CLIENT_SECRET_NAME="${IDP_CLIENT_SECRET_NAME:-${PREFIX}-idp-client-secret}" - if aws secretsmanager describe-secret --secret-id "$IDP_CLIENT_SECRET_NAME" \ + local secret_name="${!name_var:-$default_name}" + # Exported, not just assigned: app.py must see the name even on CDK calls + # that carry no context args (the bootstrap probe synthesizes the app too). + printf -v "$name_var" '%s' "$secret_name" + export "${name_var?}" + if aws secretsmanager describe-secret --secret-id "$secret_name" \ --region "$AWS_REGION" &>/dev/null; then - # Secret already exists — update the value (IdP secrets rotate). - if ! aws secretsmanager put-secret-value --secret-id "$IDP_CLIENT_SECRET_NAME" \ - --secret-string "$IDP_CLIENT_SECRET" --region "$AWS_REGION" &>/dev/null; then - log_error "Failed to update Secrets Manager secret '$IDP_CLIENT_SECRET_NAME'." + # Secret already exists — update the value (client secrets rotate). + if ! aws secretsmanager put-secret-value --secret-id "$secret_name" \ + --secret-string "$value" --region "$AWS_REGION" &>/dev/null; then + log_error "Failed to update Secrets Manager secret '$secret_name'." log_error "Check IAM permissions for secretsmanager:PutSecretValue and retry." exit 1 fi - log_info "✓ IdP client secret: updated in Secrets Manager ($IDP_CLIENT_SECRET_NAME)" + log_info "✓ $label secret: updated in Secrets Manager ($secret_name)" else - if ! aws secretsmanager create-secret --name "$IDP_CLIENT_SECRET_NAME" \ - --secret-string "$IDP_CLIENT_SECRET" --region "$AWS_REGION" &>/dev/null; then - log_error "Failed to create Secrets Manager secret '$IDP_CLIENT_SECRET_NAME'." + if ! aws secretsmanager create-secret --name "$secret_name" \ + --secret-string "$value" --region "$AWS_REGION" &>/dev/null; then + log_error "Failed to create Secrets Manager secret '$secret_name'." log_error "Check IAM permissions for secretsmanager:CreateSecret and retry." exit 1 fi - log_info "✓ IdP client secret: stored in Secrets Manager ($IDP_CLIENT_SECRET_NAME)" + log_info "✓ $label secret: stored in Secrets Manager ($secret_name)" fi # Plaintext is no longer needed — only the secret name is passed to CDK. - unset IDP_CLIENT_SECRET + unset "$value_var" +} + +upsert_idp_secret() { + upsert_oauth_secret IDP_CLIENT_SECRET IDP_CLIENT_SECRET_NAME \ + "${PREFIX}-idp-client-secret" "IdP client" +} + +# 3LO providers (module 4): GOOGLE/GITHUB/NOTION_CLIENT_SECRET in the +# environment is moved to Secrets Manager the same way — app.py refuses the +# plaintext form outright. +upsert_3lo_secrets() { + upsert_oauth_secret GOOGLE_CLIENT_SECRET GOOGLE_CLIENT_SECRET_NAME \ + "${PREFIX}-google-oauth-secret" "Google OAuth" + upsert_oauth_secret GITHUB_CLIENT_SECRET GITHUB_CLIENT_SECRET_NAME \ + "${PREFIX}-github-oauth-secret" "GitHub OAuth" + upsert_oauth_secret NOTION_CLIENT_SECRET NOTION_CLIENT_SECRET_NAME \ + "${PREFIX}-notion-oauth-secret" "Notion OAuth" } # ═══════════════════════════════════════════════════════════════ @@ -541,6 +570,14 @@ build_context_args() { [ -n "${IDP_CLIENT_SECRET_NAME:-}" ] && CONTEXT_ARGS+=(-c "idp_client_secret_name=${IDP_CLIENT_SECRET_NAME}") [ -n "${IDP_ISSUER_URL:-}" ] && CONTEXT_ARGS+=(-c "idp_issuer_url=${IDP_ISSUER_URL}") + # 3LO provider config — same rule: secret NAMES only (upsert_3lo_secrets). + [ -n "${GOOGLE_CLIENT_ID:-}" ] && CONTEXT_ARGS+=(-c "google_client_id=${GOOGLE_CLIENT_ID}") + [ -n "${GOOGLE_CLIENT_SECRET_NAME:-}" ] && CONTEXT_ARGS+=(-c "google_client_secret_name=${GOOGLE_CLIENT_SECRET_NAME}") + [ -n "${GITHUB_CLIENT_ID:-}" ] && CONTEXT_ARGS+=(-c "github_client_id=${GITHUB_CLIENT_ID}") + [ -n "${GITHUB_CLIENT_SECRET_NAME:-}" ] && CONTEXT_ARGS+=(-c "github_client_secret_name=${GITHUB_CLIENT_SECRET_NAME}") + [ -n "${NOTION_CLIENT_ID:-}" ] && CONTEXT_ARGS+=(-c "notion_client_id=${NOTION_CLIENT_ID}") + [ -n "${NOTION_CLIENT_SECRET_NAME:-}" ] && CONTEXT_ARGS+=(-c "notion_client_secret_name=${NOTION_CLIENT_SECRET_NAME}") + # Feature flags from profile [ -n "${ENABLE_NETWORKING:-}" ] && CONTEXT_ARGS+=(-c "enable_networking=${ENABLE_NETWORKING}") [ -n "${ENABLE_SECURITY:-}" ] && CONTEXT_ARGS+=(-c "enable_security=${ENABLE_SECURITY}") @@ -827,9 +864,13 @@ fi cd "$PROJECT_DIR" -# If IDP_CLIENT_SECRET came from the environment, move it into Secrets Manager -# before any context args are built (plaintext never reaches the CDK CLI). -[ "$DRY_RUN" = "1" ] || upsert_idp_secret +# If IDP_CLIENT_SECRET or a 3LO *_CLIENT_SECRET came from the environment, move +# it into Secrets Manager before any context args are built (plaintext never +# reaches the CDK CLI). +if [ "$DRY_RUN" != "1" ]; then + upsert_idp_secret + upsert_3lo_secrets +fi # Build CDK context args (populates the CONTEXT_ARGS array) build_context_args diff --git a/stacks/identity_stack.py b/stacks/identity_stack.py index ed267ae..8cfc7e8 100644 --- a/stacks/identity_stack.py +++ b/stacks/identity_stack.py @@ -27,11 +27,11 @@ def __init__( gateway_m2m_client_secret: cdk.SecretValue, cognito_discovery_url: str, google_client_id: str = "", - google_client_secret: str = "", + google_client_secret_name: str = "", github_client_id: str = "", - github_client_secret: str = "", + github_client_secret_name: str = "", notion_client_id: str = "", - notion_client_secret: str = "", + notion_client_secret_name: str = "", **kwargs, ): super().__init__(scope, id, **kwargs) @@ -39,6 +39,35 @@ def __init__( prefix = f"{project_name}-{environment}" self._provider_arns: dict[str, str] = {} + # 3LO secrets arrive as Secrets Manager secret NAMES, never values — + # the plaintext used to be rendered verbatim into + # customOAuth2ProviderConfig.clientSecret in the synthesized template. + # Same fail-fast contract as the auth stack's IdP secret. + for vendor, cid, secret_name in ( + ("google", google_client_id, google_client_secret_name), + ("github", github_client_id, github_client_secret_name), + ("notion", notion_client_id, notion_client_secret_name), + ): + if cid and not secret_name: + raise ValueError( + f"{vendor}_client_id is set but '{vendor}_client_secret_name' is missing " + "(the name of a Secrets Manager secret holding the OAuth client secret). " + "Store the secret first, e.g.:\n" + f" aws secretsmanager create-secret --name {prefix}-{vendor}-oauth-secret " + "--secret-string ''\n" + f"then pass: -c {vendor}_client_secret_name={prefix}-{vendor}-oauth-secret\n" + "(scripts/deploy.sh does both automatically when the secret is in the " + f"environment). The plaintext '{vendor}_client_secret' key is no longer " + "supported." + ) + + def _resolve(secret_name: str) -> str: + # Renders a {{resolve:secretsmanager:...}} dynamic-reference TOKEN + # into the template — not the value. CloudFormation resolves it at + # deploy time, so the secret never appears in cdk.out or the + # synthesized template. Same pattern as the auth stack. + return cdk.SecretValue.secrets_manager(secret_name).unsafe_unwrap() + # ── Gateway M2M Provider (always created) ── # Agents fetch Gateway access tokens through this provider via # @requires_access_token(provider_name=..., auth_flow="M2M"). The name @@ -87,61 +116,72 @@ def __init__( value=self._gateway_provider_name, ) - # ── Google OAuth2 Provider ── + # ── 3LO Providers (Google / GitHub / Notion) ── + # Typed L1 property classes, deliberately: these blocks used to pass a + # raw dict whose top-level key spelled OAuth with a capital A (the + # model wants Oauth), and the L1 mapping silently dropped the ENTIRE + # config — the template carried + # Oauth2ProviderConfigInput: {}. Same hazard class as the web-search + # connector (docs/GATEWAY_TARGETS.md). The typed classes raise at synth + # on a wrong key instead. Scopes are not provider config: agents + # request them per token via @requires_access_token(scopes=[...]). + _P = agentcore.CfnOAuth2CredentialProvider + + # Google and GitHub are vendor-known: the Token Vault knows their + # endpoints, so config is just the client pair. if google_client_id: - google_provider = agentcore.CfnOAuth2CredentialProvider( + google_provider = _P( self, "GoogleOAuth", name=f"{prefix}-google-oauth", credential_provider_vendor="GoogleOauth2", - oauth2_provider_config_input={ - "customOAuth2ProviderConfig": { - "oauthDiscoveryUrl": "https://accounts.google.com/.well-known/openid-configuration", - "clientId": google_client_id, - "clientSecret": google_client_secret, - "scopes": [ - "https://www.googleapis.com/auth/calendar", - "https://www.googleapis.com/auth/gmail.readonly", - "https://www.googleapis.com/auth/drive.readonly", - ], - }, - }, + oauth2_provider_config_input=_P.Oauth2ProviderConfigInputProperty( + google_oauth2_provider_config=_P.GoogleOauth2ProviderConfigInputProperty( + client_id=google_client_id, + client_secret=_resolve(google_client_secret_name), + ), + ), ) self._provider_arns["google"] = google_provider.attr_credential_provider_arn - # ── GitHub OAuth2 Provider ── if github_client_id: - github_provider = agentcore.CfnOAuth2CredentialProvider( + github_provider = _P( self, "GitHubOAuth", name=f"{prefix}-github-oauth", credential_provider_vendor="GithubOauth2", - oauth2_provider_config_input={ - "customOAuth2ProviderConfig": { - "oauthDiscoveryUrl": "https://github.com/.well-known/openid-configuration", - "clientId": github_client_id, - "clientSecret": github_client_secret, - "scopes": ["repo", "read:user", "user:email"], - }, - }, + oauth2_provider_config_input=_P.Oauth2ProviderConfigInputProperty( + github_oauth2_provider_config=_P.GithubOauth2ProviderConfigInputProperty( + client_id=github_client_id, + client_secret=_resolve(github_client_secret_name), + ), + ), ) self._provider_arns["github"] = github_provider.attr_credential_provider_arn - # ── Notion OAuth2 Provider ── + # Notion has no vendor config — CustomOauth2 with explicit endpoints, + # the same shape as the (deployed and working) gateway M2M provider. + # Notion publishes no OIDC discovery document, so the endpoints are + # spelled out instead of discovered. if notion_client_id: - notion_provider = agentcore.CfnOAuth2CredentialProvider( + notion_provider = _P( self, "NotionOAuth", name=f"{prefix}-notion-oauth", - credential_provider_vendor="Custom", - oauth2_provider_config_input={ - "customOAuth2ProviderConfig": { - "oauthDiscoveryUrl": "https://api.notion.com/.well-known/openid-configuration", - "clientId": notion_client_id, - "clientSecret": notion_client_secret, - "scopes": ["read_content", "read_user"], - }, - }, + credential_provider_vendor="CustomOauth2", + oauth2_provider_config_input=_P.Oauth2ProviderConfigInputProperty( + custom_oauth2_provider_config=_P.CustomOauth2ProviderConfigInputProperty( + oauth_discovery=_P.Oauth2DiscoveryProperty( + authorization_server_metadata=_P.Oauth2AuthorizationServerMetadataProperty( + issuer="https://api.notion.com", + authorization_endpoint="https://api.notion.com/v1/oauth/authorize", + token_endpoint="https://api.notion.com/v1/oauth/token", + ), + ), + client_id=notion_client_id, + client_secret=_resolve(notion_client_secret_name), + ), + ), ) self._provider_arns["notion"] = notion_provider.attr_credential_provider_arn diff --git a/tests/test_3lo_providers.py b/tests/test_3lo_providers.py new file mode 100644 index 0000000..d95761a --- /dev/null +++ b/tests/test_3lo_providers.py @@ -0,0 +1,65 @@ +"""3LO credential providers: secrets by name, config that actually renders. + +Static guards in the style of test_web_search_target.py (the CI test job has no +aws_cdk, so these parse source). Two defects, both found live: + +1. Client secrets were rendered verbatim into the synthesized template + (customOAuth2ProviderConfig.clientSecret) from -c context / env plaintext. + Now they travel as Secrets Manager NAMES and render as + {{resolve:secretsmanager:...}} dynamic references. +2. The provider config was passed as a raw dict whose top-level key + ("customOAuth2ProviderConfig", capital OA) did not match the CloudFormation + model — the L1 mapping silently dropped the whole block and the template + carried Oauth2ProviderConfigInput: {}. The providers could never have + worked. Typed property classes raise at synth instead. +""" + +from pathlib import Path + +ROOT = Path(__file__).resolve().parents[1] +IDENTITY_SRC = (ROOT / "stacks" / "identity_stack.py").read_text() +APP_SRC = (ROOT / "app.py").read_text() +DEPLOY_SRC = (ROOT / "scripts" / "deploy.sh").read_text() + + +def test_no_plaintext_secret_parameters(): + # The stack accepts secret NAMES only. A *_client_secret parameter + # reintroduces the template leak. + for vendor in ("google", "github", "notion"): + assert f"{vendor}_client_secret_name" in IDENTITY_SRC + assert f"{vendor}_client_secret:" not in IDENTITY_SRC # old param form + + +def test_secrets_render_as_dynamic_references(): + assert "SecretValue.secrets_manager" in IDENTITY_SRC + # unsafe_unwrap renders the TOKEN, not the value — same as the auth stack. + assert "unsafe_unwrap" in IDENTITY_SRC + + +def test_no_raw_dict_provider_config(): + # The raw-dict key that silently dropped the whole config (quoted = used + # as a dict key; the comments explaining the defect may name it bare). + assert '"customOAuth2ProviderConfig"' not in IDENTITY_SRC + # Typed property classes are the load-bearing replacement. + for prop in ( + "GoogleOauth2ProviderConfigInputProperty", + "GithubOauth2ProviderConfigInputProperty", + "CustomOauth2ProviderConfigInputProperty", + ): + assert prop in IDENTITY_SRC + + +def test_app_rejects_plaintext_and_reads_names(): + assert "no longer supported" in APP_SRC + for vendor in ("google", "github", "notion"): + assert f"{vendor}_client_secret_name" in APP_SRC + # The rejection must cover the env-var spelling too. + assert "_CLIENT_SECRET" in APP_SRC + + +def test_deploy_upserts_and_passes_names_only(): + assert "upsert_3lo_secrets" in DEPLOY_SRC + for vendor in ("GOOGLE", "GITHUB", "NOTION"): + assert f"{vendor}_CLIENT_SECRET_NAME" in DEPLOY_SRC + # The plaintext env var must never be a context arg. + assert f'-c "{vendor.lower()}_client_secret=' not in DEPLOY_SRC diff --git a/tests/test_platform_config.py b/tests/test_platform_config.py index 02662f5..38a669d 100644 --- a/tests/test_platform_config.py +++ b/tests/test_platform_config.py @@ -188,13 +188,11 @@ def test_to_env_omits_empty_values(): def test_app_py_resolves_through_cfg(): """Static guard: every legacy `try_get_context(...) or environ.get(...)` lookup in app.py must go through cfg() so platform.yaml participates. - OAuth provider secrets are the deliberate exception (secrets never live - in the config file).""" + The 3LO plaintext-secret rejection loop is the deliberate exception: it + probes keys that must NOT resolve (cfg would legitimize them).""" src = (REPO / "app.py").read_text() allowed = ( - "google_client", - "github_client", - "notion_client", + "_vendor", # the plaintext-rejection loop "platform_config", "region", # region merges CDK_DEFAULT_REGION explicitly "CDK_DEFAULT_ACCOUNT",