🔑 fix: Decode Action OAuth Credentials After Decryption - #14698
Open
lidonius1122 wants to merge 1 commit into
Open
🔑 fix: Decode Action OAuth Credentials After Decryption#14698lidonius1122 wants to merge 1 commit into
lidonius1122 wants to merge 1 commit into
Conversation
Action client credentials are URL-encoded before encryption, but the OAuth token exchange decrypted them with `decryptV2` alone, sending values like `abc%40domain.com` to the provider and causing 401s for secrets containing `@`, `+`, `=`, `/` or `:`. Decoding falls back to the raw decrypted value so legacy secrets stored before 299cabd (which contain an unescaped `%`) still work. Fixes danny-avila#14636
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.
Summary
Fixes #14636
Action OAuth client credentials are URL-encoded before encryption:
encryptSensitiveValueinapi/server/services/ActionService.jsappliesencodeURIComponent()beforeencryptV2. The decrypt helpers inActionService.jsreverse both steps, butgetAccessTokenandrefreshAccessTokeninpackages/api/src/oauth/tokens.tsdecrypted withdecryptV2alone — so any client ID or secret containing special characters (@,+,=,/,:) was sent to the provider's token endpoint still URL-encoded (e.g.abc%40domain.cominstead ofabc@domain.com), causing 401s. Both exchange methods were affected:default_post(body params) andbasic_auth_header(base64 header).This PR adds a
decryptSensitiveValuehelper intokens.tsthat decodes after decrypting and uses it at all four call sites.Backward compatibility: encoding-before-encryption was introduced in 299cabd (March 2025); credentials stored before that are encrypted without it. A legacy secret containing a raw
%would makedecodeURIComponentthrowURIError, so the decode is wrapped in try/catch and falls back to the raw decrypted value.api/server/services/ActionService.jsis untouched.Change Type
Testing
Added 6 test cases to
packages/api/src/oauth/tokens.spec.ts(existing mock/helper patterns):getAccessToken+refreshAccessTokenwithdefault_post: request body contains the decodedclient_id/client_secretgetAccessToken+refreshAccessTokenwithbasic_auth_header:Authorizationheader base64-decodes todecodedId:decodedSecret%: no throw, passed through unchanged (both paths)Verified the decode assertions fail against the previous code (reverting the call sites to
decryptV2makes the 4 decode tests fail), so the tests pin the actual bug.cd packages/api && npx jest src/oauth→ 5 suites, 68 tests passednpx tsc --noEmit -p packages/api→ cleannpx eslinton both touched files → cleanChecklist