fix(security): remove committed Apple Sign In private key from E2E config - #14986
Open
Simone319 wants to merge 1 commit into
Open
fix(security): remove committed Apple Sign In private key from E2E config#14986Simone319 wants to merge 1 commit into
Simone319 wants to merge 1 commit into
Conversation
…nfig The Sign in with Apple private key committed across the E2E test configuration is a real, cryptographically valid P-256 (ES256) key, not an inert placeholder: `openssl pkey` parses it and a sign/verify round trip succeeds. Per the in-repo comments it was deliberately invalidated Apple-side, but it has been public since May 2021, so the literal should not remain in source. None of these call sites need a specific registered key. APPLE_APP_ID (com.fake.app), APPLE_TEAM_ID and APPLE_KEY_ID are all fake and the committed key is revoked, yet the tests pass today - so no Apple round trip occurs and Cognito only performs config-time structural validation. A key generated at run time satisfies every site. Add getEphemeralApplePrivateKey() to amplify-e2e-core/src/utils/envVars.ts, which generates a memoized P-256 key and flattens it to a single line (callers type this value into interactive CLI prompts, where a newline would register as an Enter keypress). - envVars.ts: mock branch uses the generated key; the getEnv branch prefers an injected APPLE_PRIVATE_KEY_2 and falls back to the generated key, so removing the literal from CI config cannot fail the run. - auth-utils.ts: appleAppPrivateKey uses the same helper. - sample.env: distinct placeholders for APPLE_PRIVATE_KEY and APPLE_PRIVATE_KEY_2, which were previously byte-identical, plus a note that only APPLE_PRIVATE_KEY_2 is read. - codebuild_specs (5 files): drop the hardcoded APPLE_PRIVATE_KEY_2 value. Test-only and CI-config-only; no runtime or customer code path is affected.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The finding
Secret scanning (Mirador
acat-bosco/rsa-private-key) flagged a committed private-key literal inextract-apple-private-key.test.ts. On investigation, the value is not an inert placeholder —it is a real, cryptographically valid P-256 (ES256) private key:
openssl pkey -inform DERparses it:ASN1 OID: prime256v1,NIST CURVE: P-256Verified OK)public for ~5 years, on both
devandmainAn earlier triage of mine concluded the value was unparseable. That was wrong: it was tested with
openssl ec, which expects SEC1 format, while this key is PKCS#8 — the parse error was a formatmismatch, not invalid key material. Correcting that here.
Per the in-repo comments (
"the following keys are invalidated but they pass Cognito validation")the key was deliberately revoked Apple-side, which is plausible — but a code comment is not
verification, and valid key material should not sit in a public repo regardless.
Sites not covered by #14985
PR #14985 removes the literal from the one file Mirador flagged. The same key is committed at
9 further sites across 8 files, none of which were flagged (the 4-dash
----BEGINmarkerslikely evade the scanner's regex — a detection gap worth fixing separately):
packages/amplify-e2e-core/src/utils/envVars.tsgetEnvfallback use a generated keypackages/amplify-e2e-core/src/utils/auth-utils.tsappleAppPrivateKeyuses the same helperpackages/amplify-e2e-tests/sample.envcodebuild_specs/run_e2e_tests_linux.ymlAPPLE_PRIVATE_KEY_2codebuild_specs/run_e2e_tests_linux_split.ymlcodebuild_specs/run_e2e_tests_windows.ymlcodebuild_specs/run_e2e_tests_windows_split.ymlcodebuild_specs/amplify_console_integration_tests.ymlApproach: generate at run time — no CI secret required
No new CI secret needs to be configured.
APPLE_APP_ID(com.fake.app),APPLE_TEAM_IDandAPPLE_KEY_IDare all fake values, and the committed key is revoked — yet these tests pass today.That proves no Apple round trip occurs: Cognito only validates the key's structure when the
provider is configured. So no site needs a specific registered key, only a well-formed one.
Injecting a key from Secrets Manager was considered and rejected: it would add a manual setup step,
an IAM change and a CI-breakage window, and would still store key material — for no benefit, since
nothing depends on a particular key.
getEphemeralApplePrivateKey()(new, inenvVars.ts) generates a memoized P-256 key and flattensthe PEM to a single line. The single-line form is required: callers type this value into
interactive CLI prompts via
.send()/.sendLine(), so an embedded newline would register as anEnter keypress and break the prompt.
extractApplePrivateKeystrips whitespace and isdelimiter-dash agnostic, so a single-line PEM parses correctly.
Escape hatch preserved: the
getEnvbranch still prefers an injectedAPPLE_PRIVATE_KEY_2and onlyfalls back to the generated key when unset. Anyone who does want to pin a specific key can inject
one from Secrets Manager with no code change. The now-redundant
APPLE_PRIVATE_KEYentry was alsodropped from
missingVars, so removing the literal from the CodeBuild specs cannot fail a run.APPLE_PRIVATE_KEY/_2duplicationIn
sample.envthese two were byte-identical, so any test expecting two distinct keys wassilently exercising the same one. They are now distinct placeholders, with a note that
getSocialProviders()reads onlyAPPLE_PRIVATE_KEY_2.Verification
tsc --noEmit --strictonenvVars.ts: cleanprettier --checkon all 7 TS/YAML files:All matched files use Prettier code style!APPLE_PRIVATE_KEY_2absentextractApplePrivateKeyround trip yielding a 138-byte P-256 PKCS#8 body, idempotence, generated keyvalidity (
sign → verify), mock-branch fields intact,getEnvprecedence (injected value wins),fallback when unset (no throw), and that genuinely missing vars still throw
Follow-up (not in this PR)
2QLEWNDK6K/ key2QLZXKYJ8J; rotate if not.