fix: hide new include-ims-annotation secrets#83
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
pru55e11
left a comment
There was a problem hiding this comment.
Issue found by cursor:
One issue with the test: the assertion not.toEqual(expect.stringContaining('secret')) will fail when run in generated projects. After hiding, the output JSON still contains the key name client_secret, which includes the substring secret. So stringContaining('secret') matches and the not assertion fails.
This passes CI here because the template test isn't executed in this repo, but developers will hit it when they run tests in their generated apps.
Suggested fix:
expect(utils.stringParameters(params)).not.toEqual(expect.stringContaining('"secret"'))
(Checking for the quoted value "secret" instead of the bare substring secret avoids matching the key name client_secret.)
There was a problem hiding this comment.
Pull request overview
This PR updates the common action-template logging utility to avoid leaking IMS S2S credentials (specifically client_secret) when action input params are stringified for debug logging.
Changes:
- Redacts
params.__ims_oauth_s2s.client_secretinstringParameters. - Adds a unit test asserting IMS credential redaction.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| lib/common-templates/utils.js | Redacts client_secret when serializing params for logs. |
| lib/common-templates/utils.test.js | Adds coverage for IMS credential redaction in stringParameters. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Description
relates to adobe/aio-lib-runtime#224
Related Issue
Motivation and Context
How Has This Been Tested?
Screenshots (if appropriate):
Types of changes
Checklist: