-
Notifications
You must be signed in to change notification settings - Fork 5.6k
[FEATURE] Programmatically Rotate AWS seller credentials #19559
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub. 1 Skipped Deployment
|
WalkthroughAdds a new "Rotate Client Secret" action and corresponding app API, introduces SQS receive/delete helpers and a new AWS SQS source, and bumps multiple component action/source/package version strings. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Action as Rotate Action
participant App as amazon_selling_partner.app
participant SPAPI as Amazon SP‑API
User->>Action: invoke action
Action->>App: rotateClientSecret({ $ })
Note right of App `#E8F0FE`: Adds JSON headers\nPOST /applications/2023-11-30/clientSecret
App->>SPAPI: POST rotation request (with headers)
SPAPI-->>App: rotation response
App-->>Action: response
Note over Action,User `#E8F6EA`: export summary\nreturn { success: true, response }
Action-->>User: result
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Pre-merge checks and finishing touches❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
b056ab1 to
a4b89ef
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
📜 Review details
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (2)
components/amazon_selling_partner/actions/rotate-client-secret/rotate-client-secret.mjscomponents/amazon_selling_partner/amazon_selling_partner.app.mjs
🧰 Additional context used
🧬 Code graph analysis (1)
components/amazon_selling_partner/actions/rotate-client-secret/rotate-client-secret.mjs (1)
components/amazon_selling_partner/amazon_selling_partner.app.mjs (1)
response(167-169)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
- GitHub Check: Verify TypeScript components
- GitHub Check: pnpm publish
- GitHub Check: Lint Code Base
- GitHub Check: Publish TypeScript components
🔇 Additional comments (3)
components/amazon_selling_partner/actions/rotate-client-secret/rotate-client-secret.mjs (2)
1-33: Excellent user experience with comprehensive documentation and prerequisites.The action includes:
- Clear description with AWS documentation link
- Informative prerequisites alert explaining SQS queue requirement and 7-day expiration
- Proper security with
secret: truefor clientSecret- Appropriate annotations
34-66: Well-structured action flow with good error handling.The two-step flow correctly obtains an LWA access token and then rotates the client secret. The explicit check for
access_tokenat line 48 provides clear error messaging if authentication fails.The return structure is comprehensive, including success status, message, expiration warning, and the raw API response.
components/amazon_selling_partner/amazon_selling_partner.app.mjs (1)
215-226: API version is correct.The endpoint uses version
2023-11-30, which is the current version of the Amazon Selling Partner Application Management API for the rotateClientSecret operation.
components/amazon_selling_partner/actions/rotate-client-secret/rotate-client-secret.mjs
Outdated
Show resolved
Hide resolved
components/amazon_selling_partner/amazon_selling_partner.app.mjs
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
♻️ Duplicate comments (1)
components/amazon_selling_partner/amazon_selling_partner.app.mjs (1)
198-214: Critical: Incorrect OAuth scope prevents rotation from working.The default scope
"sellingpartnerapi::migration"is incorrect. The Application Management API requires"sellingpartnerapi::client_credential:rotation"for client secret rotation operations. This will cause authentication failures when the action is executed.🔎 Fix the default scope parameter
async getLWAAccessToken({ - $ = this, clientId, clientSecret, grantType = "client_credentials", scope = "sellingpartnerapi::migration", + $ = this, clientId, clientSecret, grantType = "client_credentials", scope = "sellingpartnerapi::client_credential:rotation", }) {
📜 Review details
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (12)
components/amazon_selling_partner/actions/check-fba-inventory-levels/check-fba-inventory-levels.mjscomponents/amazon_selling_partner/actions/fetch-orders-by-date-range/fetch-orders-by-date-range.mjscomponents/amazon_selling_partner/actions/generate-sales-inventory-reports/generate-sales-inventory-reports.mjscomponents/amazon_selling_partner/actions/get-order-details/get-order-details.mjscomponents/amazon_selling_partner/actions/list-inbound-shipments/list-inbound-shipments.mjscomponents/amazon_selling_partner/actions/optimize-product-pricing/optimize-product-pricing.mjscomponents/amazon_selling_partner/actions/retrieve-sales-performance-reports/retrieve-sales-performance-reports.mjscomponents/amazon_selling_partner/actions/rotate-client-secret/rotate-client-secret.mjscomponents/amazon_selling_partner/amazon_selling_partner.app.mjscomponents/amazon_selling_partner/package.jsoncomponents/amazon_selling_partner/sources/new-inbound-shipment-to-fba-created/new-inbound-shipment-to-fba-created.mjscomponents/amazon_selling_partner/sources/new-order-created/new-order-created.mjs
🧰 Additional context used
🧬 Code graph analysis (1)
components/amazon_selling_partner/actions/rotate-client-secret/rotate-client-secret.mjs (2)
components/amazon_selling_partner/actions/get-order-details/get-order-details.mjs (1)
response(33-36)components/amazon_selling_partner/amazon_selling_partner.app.mjs (1)
response(167-169)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: Lint Code Base
- GitHub Check: Publish TypeScript components
- GitHub Check: Verify TypeScript components
🔇 Additional comments (14)
components/amazon_selling_partner/package.json (1)
3-3: LGTM: Appropriate version bump for new feature.The minor version increment from 0.2.0 to 0.3.0 correctly follows semantic versioning for the addition of the new credential rotation feature.
components/amazon_selling_partner/actions/get-order-details/get-order-details.mjs (1)
7-7: LGTM: Coordinated version bump.Metadata-only update aligning with the package version increment. No functional changes.
components/amazon_selling_partner/actions/check-fba-inventory-levels/check-fba-inventory-levels.mjs (1)
7-7: LGTM: Coordinated version bump.Metadata-only update aligning with the package version increment. No functional changes.
components/amazon_selling_partner/sources/new-order-created/new-order-created.mjs (1)
8-8: LGTM: Coordinated version bump.Metadata-only update aligning with the package version increment. No functional changes.
components/amazon_selling_partner/actions/retrieve-sales-performance-reports/retrieve-sales-performance-reports.mjs (1)
8-8: LGTM: Coordinated version bump.Metadata-only update aligning with the package version increment. No functional changes.
components/amazon_selling_partner/actions/generate-sales-inventory-reports/generate-sales-inventory-reports.mjs (1)
7-7: LGTM: Coordinated version bump.Metadata-only update aligning with the package version increment. No functional changes.
components/amazon_selling_partner/actions/fetch-orders-by-date-range/fetch-orders-by-date-range.mjs (1)
7-7: LGTM: Coordinated version bump.Metadata-only update aligning with the package version increment. No functional changes.
components/amazon_selling_partner/actions/list-inbound-shipments/list-inbound-shipments.mjs (1)
7-7: LGTM: Coordinated version bump.Metadata-only update aligning with the package version increment. No functional changes.
components/amazon_selling_partner/actions/optimize-product-pricing/optimize-product-pricing.mjs (1)
7-7: LGTM: Version bump is appropriate.The version increment aligns with the broader package updates in this PR.
components/amazon_selling_partner/amazon_selling_partner.app.mjs (1)
215-226: LGTM: rotateClientSecret implementation is correct.The method properly constructs the POST request to the Application Management API endpoint with the required access token header.
components/amazon_selling_partner/actions/rotate-client-secret/rotate-client-secret.mjs (3)
3-21: LGTM: Well-documented prerequisites and metadata.The action metadata is clear, and the prerequisites alert provides essential setup guidance including SQS queue registration requirements and the 7-day credential expiration window.
22-32: LGTM: Props correctly configured.The Client ID and Client Secret props are properly defined with clear descriptions, and the secret prop is correctly marked as sensitive.
58-65: LGTM: Response structure is informative and user-friendly.The action provides a clear summary, includes the critical 7-day expiration warning, and returns a well-structured response payload.
components/amazon_selling_partner/sources/new-inbound-shipment-to-fba-created/new-inbound-shipment-to-fba-created.mjs (1)
8-8: LGTM: Version bump is appropriate.The version increment aligns with the broader package updates in this PR.
components/amazon_selling_partner/actions/rotate-client-secret/rotate-client-secret.mjs
Outdated
Show resolved
Hide resolved
a4b89ef to
1fd18e3
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
📜 Review details
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (14)
components/amazon_selling_partner/actions/check-fba-inventory-levels/check-fba-inventory-levels.mjscomponents/amazon_selling_partner/actions/fetch-orders-by-date-range/fetch-orders-by-date-range.mjscomponents/amazon_selling_partner/actions/generate-sales-inventory-reports/generate-sales-inventory-reports.mjscomponents/amazon_selling_partner/actions/get-order-details/get-order-details.mjscomponents/amazon_selling_partner/actions/list-inbound-shipments/list-inbound-shipments.mjscomponents/amazon_selling_partner/actions/optimize-product-pricing/optimize-product-pricing.mjscomponents/amazon_selling_partner/actions/retrieve-sales-performance-reports/retrieve-sales-performance-reports.mjscomponents/amazon_selling_partner/actions/rotate-client-secret/rotate-client-secret.mjscomponents/amazon_selling_partner/amazon_selling_partner.app.mjscomponents/amazon_selling_partner/package.jsoncomponents/amazon_selling_partner/sources/new-inbound-shipment-to-fba-created/new-inbound-shipment-to-fba-created.mjscomponents/amazon_selling_partner/sources/new-order-created/new-order-created.mjscomponents/aws/common/common-sqs.mjscomponents/aws/sources/new-sqs-event/new-sqs-event.mjs
🧰 Additional context used
🧬 Code graph analysis (3)
components/aws/common/common-sqs.mjs (1)
components/aws/sources/new-sqs-event/new-sqs-event.mjs (1)
params(66-70)
components/amazon_selling_partner/actions/rotate-client-secret/rotate-client-secret.mjs (3)
components/amazon_selling_partner/actions/get-order-details/get-order-details.mjs (1)
response(33-36)components/amazon_selling_partner/amazon_selling_partner.app.mjs (1)
response(169-171)components/aws/common/common-sqs.mjs (1)
response(25-27)
components/aws/sources/new-sqs-event/new-sqs-event.mjs (2)
components/amazon_selling_partner/sources/new-inbound-shipment-to-fba-created/new-inbound-shipment-to-fba-created.mjs (1)
meta(54-54)components/amazon_selling_partner/sources/new-order-created/new-order-created.mjs (1)
meta(42-42)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
- GitHub Check: pnpm publish
- GitHub Check: Verify TypeScript components
- GitHub Check: Publish TypeScript components
- GitHub Check: Lint Code Base
🔇 Additional comments (19)
components/amazon_selling_partner/actions/list-inbound-shipments/list-inbound-shipments.mjs (1)
7-7: LGTM: Version bump for coordinated release.The version increment aligns with the package-wide release for the new rotate client secret feature.
components/amazon_selling_partner/actions/retrieve-sales-performance-reports/retrieve-sales-performance-reports.mjs (1)
8-8: LGTM: Version bump for coordinated release.components/amazon_selling_partner/package.json (1)
3-3: LGTM: Appropriate minor version bump.The 0.2.0 → 0.3.0 bump correctly reflects the addition of new functionality (rotate client secret action) per semantic versioning conventions.
components/amazon_selling_partner/actions/generate-sales-inventory-reports/generate-sales-inventory-reports.mjs (1)
7-7: LGTM: Version bump for coordinated release.components/amazon_selling_partner/actions/check-fba-inventory-levels/check-fba-inventory-levels.mjs (1)
7-7: LGTM: Version bump for coordinated release.components/amazon_selling_partner/actions/optimize-product-pricing/optimize-product-pricing.mjs (1)
7-7: LGTM: Version bump for coordinated release.components/amazon_selling_partner/actions/get-order-details/get-order-details.mjs (1)
7-7: LGTM: Version bump for coordinated release.components/amazon_selling_partner/sources/new-inbound-shipment-to-fba-created/new-inbound-shipment-to-fba-created.mjs (1)
8-8: LGTM: Version bump for coordinated release.components/amazon_selling_partner/sources/new-order-created/new-order-created.mjs (1)
8-8: LGTM: Version bump.The version increment is appropriate for package maintenance alongside other changes in this PR.
components/amazon_selling_partner/actions/fetch-orders-by-date-range/fetch-orders-by-date-range.mjs (1)
7-7: LGTM: Version bump.Metadata update aligns with the overall package versioning in this PR.
components/amazon_selling_partner/amazon_selling_partner.app.mjs (2)
108-109: LGTM: Standard JSON headers.The addition of JSON Content-Type and Accept headers is appropriate for REST API communication, particularly for the new Application Management API endpoint.
200-206: LGTM: rotateClientSecret method implementation.The method correctly wraps the Application Management API endpoint for rotating client secrets. The implementation follows the established pattern of other API methods in this module.
Note: There's a past review comment about OAuth scope configuration (
sellingpartnerapi::client_credential:rotationvssellingpartnerapi::migration). While this method implementation is correct, ensure the OAuth configuration provides the proper scope for the Application Management API. This may require verification in the authentication setup.components/aws/common/common-sqs.mjs (2)
6-7: LGTM: AWS SDK imports.The ReceiveMessageCommand and DeleteMessageCommand imports are correctly sourced from the AWS SDK and necessary for the new SQS message handling methods.
47-52: LGTM: SQS message lifecycle methods.The new
sqsReceiveMessageandsqsDeleteMessagemethods follow the established pattern in this module and correctly wrap the AWS SDK commands. The implementation is consistent with existing SQS methods.components/aws/sources/new-sqs-event/new-sqs-event.mjs (3)
1-11: LGTM: Source metadata.The source configuration is appropriate. The
dedupe: "unique"setting will prevent duplicate emissions based onMessageId, which is correct for SQS message processing.
12-46: LGTM: Props configuration.The prop definitions are well-configured:
maxNumberOfMessagesrange (1-10) aligns with SQS API limitswaitTimeSecondsrange (0-20) matches SQS long polling constraints- Defaults are sensible for typical use cases
48-56: LGTM: Metadata generation.The
generateMetamethod correctly usesMessageIdfor deduplication. UsingDate.now()for the timestamp is appropriate as it reflects processing time rather than message creation time.components/amazon_selling_partner/actions/rotate-client-secret/rotate-client-secret.mjs (2)
1-22: LGTM: Action metadata and documentation.The action is well-documented with:
- Clear description of the operation and its consequences
- Prominent warning about the 7-day credential expiry
- Prerequisites alert about the required SQS queue setup
- Appropriate annotations for the operation type
23-34: LGTM: Clean action implementation.The
runmethod correctly:
- Delegates to the
rotateClientSecretAPI method- Returns a structured response with the API result
- Provides user-friendly success feedback
Note: Past review comments referenced authentication code that's no longer present in this simplified implementation. The current version appropriately relies on the OAuth token provided by the platform's authentication mechanism. Ensure the OAuth app configuration includes the correct scope (
sellingpartnerapi::client_credential:rotation) for the Application Management API.
1fd18e3 to
344c818
Compare
GTFalcao
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM! Though the version numbers need to be adjusted
components/amazon_selling_partner/actions/rotate-client-secret/rotate-client-secret.mjs
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
♻️ Duplicate comments (1)
components/aws/sources/new-sqs-event/new-sqs-event.mjs (1)
85-110: Unresolved: Deletion error handling still missing.As noted in the previous review, the message deletion (lines 105-108) lacks error handling. If deletion fails, it will throw and halt processing of remaining messages in the batch, reducing reliability.
This issue was previously flagged and agreed upon but remains unresolved.
🔎 Recommended fix: wrap deletion in try-catch
this.$emit(parsedMessage, meta); // Delete the message from the queue after emitting - await this.sqsDeleteMessage({ - QueueUrl: queueUrl, - ReceiptHandle: message.ReceiptHandle, - }); + try { + await this.sqsDeleteMessage({ + QueueUrl: queueUrl, + ReceiptHandle: message.ReceiptHandle, + }); + } catch (error) { + console.log(`Failed to delete message ${message.MessageId}: ${error.message}`); + // Message will become visible again after visibility timeout + } }
📜 Review details
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (2)
components/amazon_selling_partner/actions/rotate-client-secret/rotate-client-secret.mjscomponents/aws/sources/new-sqs-event/new-sqs-event.mjs
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-10-20T01:01:02.970Z
Learnt from: js07
Repo: PipedreamHQ/pipedream PR: 18744
File: components/slack_v2/actions/send-large-message/send-large-message.mjs:49-64
Timestamp: 2025-10-20T01:01:02.970Z
Learning: In components/slack_v2/actions/send-large-message/send-large-message.mjs, the metadata_event_payload prop is typed as string, so the code only needs to handle string-to-JSON parsing and does not need to handle object inputs.
Applied to files:
components/aws/sources/new-sqs-event/new-sqs-event.mjs
🧬 Code graph analysis (1)
components/aws/sources/new-sqs-event/new-sqs-event.mjs (2)
components/amazon_selling_partner/sources/new-inbound-shipment-to-fba-created/new-inbound-shipment-to-fba-created.mjs (1)
meta(54-54)components/amazon_selling_partner/sources/new-order-created/new-order-created.mjs (1)
meta(42-42)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
- GitHub Check: pnpm publish
- GitHub Check: Lint Code Base
- GitHub Check: Verify TypeScript components
- GitHub Check: Publish TypeScript components
🔇 Additional comments (7)
components/amazon_selling_partner/actions/rotate-client-secret/rotate-client-secret.mjs (3)
1-2: LGTM!The import statement is correct.
14-22: LGTM!The prerequisites alert provides helpful guidance about the destination queue requirement. The ESLint disable comments are appropriate for alert-type props.
23-34: LGTM! Clean implementation.The run function correctly delegates to the app method and follows Pipedream action patterns. The simplified implementation avoids the issues flagged in past review comments (which referenced getLWAAccessToken calls that no longer exist in this file).
components/aws/sources/new-sqs-event/new-sqs-event.mjs (4)
1-11: This file appears unrelated to the stated PR objectives.The PR objectives describe implementing programmatic rotation of AWS seller credentials (#19400), but this file introduces a new SQS message polling source. Unless SQS is used for credential rotation notifications (which is not mentioned), this change seems out of scope for this PR.
Please clarify why this SQS source is included in a credential rotation PR. If these are separate features, consider splitting them into separate PRs for easier review and maintenance.
12-47: LGTM: Props configuration.The props are correctly configured with appropriate types, constraints matching AWS SQS limits (MaxNumberOfMessages: 1-10, WaitTimeSeconds: 0-20), and sensible defaults.
48-57: LGTM: Metadata generation.The
generateMetamethod correctly usesmessage.MessageIdas the unique identifier for deduplication, with a clear summary and timestamp.
58-84: LGTM: Message retrieval logic.The message retrieval correctly constructs SQS parameters, handles optional
visibilityTimeout, and provides appropriate logging for both empty and successful polling scenarios.
| annotations: { | ||
| destructiveHint: false, | ||
| openWorldHint: true, | ||
| readOnlyHint: false, | ||
| }, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Set destructiveHint to true for credential rotation.
Credential rotation causes the old secret to expire after 7 days (as noted in the description). This is a destructive operation that can break existing integrations if users don't update their credentials in time. Setting destructiveHint: true will properly warn users about the operation's impact.
🔎 Proposed fix
annotations: {
- destructiveHint: false,
+ destructiveHint: true,
openWorldHint: true,
readOnlyHint: false,
},🤖 Prompt for AI Agents
In
components/amazon_selling_partner/actions/rotate-client-secret/rotate-client-secret.mjs
around lines 8 to 12, the action annotations mark destructiveHint as false but
this operation rotates credentials and expires the old secret after 7 days;
change destructiveHint to true to signal the destructive nature. Edit the
annotations block to set destructiveHint: true (leave other hints unchanged),
run a quick lint/type check, and ensure any tests or documentation referencing
the hint are updated if necessary.
WHY
Resolves #19400
Summary by CodeRabbit
New Features
Chores
✏️ Tip: You can customize this high-level summary in your review settings.