Skip to content

chore(ci): replace AWS static credentials with GitHub OIDC#41700

Open
mohanarpit wants to merge 225 commits intoreleasefrom
chore/replace-aws-static-creds-with-oidc
Open

chore(ci): replace AWS static credentials with GitHub OIDC#41700
mohanarpit wants to merge 225 commits intoreleasefrom
chore/replace-aws-static-creds-with-oidc

Conversation

@mohanarpit
Copy link
Copy Markdown
Member

@mohanarpit mohanarpit commented Apr 4, 2026

Summary

  • Replace long-lived AWS AWS_ACCESS_KEY_ID / AWS_SECRET_ACCESS_KEY static credentials in three GitHub Actions workflows with short-lived OIDC tokens via aws-actions/configure-aws-credentials@v4
  • Add permissions: id-token: write to each affected job to enable GitHub OIDC token issuance
  • Affected workflows: helm-release.yml, cleanup-dp.yml, on-demand-build-docker-image-deploy-preview.yml

AWS setup required (one-time, before merging)

  • Configure GitHub as an OIDC identity provider in the AWS account (if not already done)
  • Create/update IAM roles with trust policy for the GitHub OIDC provider:
    • HELM_AWS_ROLE_ARN (new secret) — role with S3 read/write access to the Helm chart bucket
    • APPSMITH_EKS_AWS_ROLE_ARN (already exists) — role with EKS access for deploy previews
  • Once roles are set up, the old secrets (HELM_AWS_ACCESS_KEY_ID, HELM_AWS_SECRET_ACCESS_KEY, APPSMITH_CI_AWS_SECRET_ACCESS_KEY_ID, APPSMITH_CI_AWS_SECRET_ACCESS_KEY) can be deleted

Test plan

  • Verify HELM_AWS_ROLE_ARN GitHub secret is created with the correct IAM role ARN
  • Verify APPSMITH_EKS_AWS_ROLE_ARN GitHub secret exists and the role has a trust policy for the GitHub OIDC provider
  • Trigger helm-release.yml via workflow_dispatch and confirm Helm chart publishes to S3 successfully
  • Trigger cleanup-dp.yml via workflow_dispatch and confirm EKS cleanup runs without credential errors
  • Open a test PR and run /build-deploy-preview to confirm deploy preview deploys successfully

🤖 Generated with Claude Code

Summary by CodeRabbit

Release Notes

  • Bug Fixes

    • Improved error reporting with better context and cause tracking for Git operations and resource access failures.
    • Enhanced permission validation for application snapshot operations with clearer error messages.
  • Chores

    • Updated GitHub Actions workflows with improved security configuration for credential handling.

btsgh and others added 30 commits October 7, 2024 16:18
Fixes issue where the detection for signup when using OAuth was not
being handled correctly.

[Slack
conversation](https://theappsmith.slack.com/archives/C02K2MZERSL/p1732600773587469?thread_ts=1732554015.110689&cid=C02K2MZERSL).


## Automation

/test sanity

### 🔍 Cypress test results
<!-- This is an auto-generated comment: Cypress test results  -->
> [!WARNING]
> Workflow run:
<https://github.com/appsmithorg/appsmith/actions/runs/12024883331>
> Commit: d53fcdf
> <a
href="https://internal.appsmith.com/app/cypress-dashboard/rundetails-65890b3c81d7400d08fa9ee5?branch=master&workflowId=12024883331&attempt=1"
target="_blank">Cypress dashboard</a>.
> Tags: @tag.Sanity
> Spec: 
> It seems like **no tests ran** 😔. We are not able to recognize it,
please check <a
href="https://github.com/appsmithorg/appsmith/actions/runs/12024883331"
target="_blank">workflow here</a>.
> <hr>Tue, 26 Nov 2024 06:16:02 UTC
<!-- end of auto-generated comment: Cypress test results  -->


## Communication
Should the DevRel and Marketing teams inform users about this change?
- [ ] Yes
- [x] No


<!-- This is an auto-generated comment: release notes by coderabbit.ai
-->

## Summary by CodeRabbit

- **New Features**
- Improved handling of user authentication success, enhancing the flow
for email verification and OAuth2 authentication.
- **Refactor**
	- Simplified the logic for determining user sign-up or login status.
- Streamlined the method for handling OAuth2 redirects, improving
clarity and maintainability.

<!-- end of auto-generated comment: release notes by coderabbit.ai -->
btsgh and others added 28 commits September 22, 2025 14:16
…1356)

Changed the download link for Google Chrome version 129.0.6668.100 in
the CI workflow to a new S3 location, ensuring continued access to the
specified version for consistent testing environments.

> [!TIP]
> _Add a TL;DR when the description is longer than 500 words or
extremely technical (helps the content, marketing, and DevRel team)._
>
> _Please also include relevant motivation and context. List any
dependencies that are required for this change. Add links to Notion,
Figma or any other documents that might be relevant to the PR._

Fixes #`Issue Number`
_or_
Fixes `Issue URL`
> [!WARNING]
> _If no issue exists, please create an issue first, and check with the
maintainers if the issue is valid._

/ok-to-test tags=""

<!-- This is an auto-generated comment: Cypress test results  -->
> [!CAUTION]
> If you modify the content in this section, you are likely to disrupt
the CI result for your PR.

<!-- end of auto-generated comment: Cypress test results  -->

Should the DevRel and Marketing teams inform users about this change?
- [ ] Yes
- [ ] No

## Description
> [!TIP]  
> _Add a TL;DR when the description is longer than 500 words or
extremely technical (helps the content, marketing, and DevRel team)._
>
> _Please also include relevant motivation and context. List any
dependencies that are required for this change. Add links to Notion,
Figma or any other documents that might be relevant to the PR._


Fixes #`Issue Number`  
_or_  
Fixes `Issue URL`
> [!WARNING]  
> _If no issue exists, please create an issue first, and check with the
maintainers if the issue is valid._

## Automation

/ok-to-test tags=""

### 🔍 Cypress test results
<!-- This is an auto-generated comment: Cypress test results  -->
> [!CAUTION]  
> If you modify the content in this section, you are likely to disrupt
the CI result for your PR.

<!-- end of auto-generated comment: Cypress test results  -->


## Communication
Should the DevRel and Marketing teams inform users about this change?
- [ ] Yes
- [ ] No
…emory Git (#41442)

## Description

EE Counterpart: appsmithorg/appsmith-ee#8468

Fixes #`Issue Number`
_or_
Fixes `Issue URL`
> [!WARNING]
> _If no issue exists, please create an issue first, and check with the
maintainers if the issue is valid._

## Automation

/ok-to-test tags="@tag.Git"

### 🔍 Cypress test results
<!-- This is an auto-generated comment: Cypress test results  -->
> [!TIP]
> 🟢 🟢 🟢 All cypress tests have passed! 🎉 🎉 🎉
> Workflow run:
<https://github.com/appsmithorg/appsmith/actions/runs/19896141793>
> Commit: 478fefe
> <a
href="https://internal.appsmith.com/app/cypress-dashboard/rundetails-65890b3c81d7400d08fa9ee5?branch=master&workflowId=19896141793&attempt=1"
target="_blank">Cypress dashboard</a>.
> Tags: `@tag.Git`
> Spec:
> <hr>Wed, 03 Dec 2025 14:41:38 UTC
<!-- end of auto-generated comment: Cypress test results  -->

## Communication
Should the DevRel and Marketing teams inform users about this change?
- [x] Yes
- [ ] No

<!-- This is an auto-generated comment: release notes by coderabbit.ai
-->
## Summary by CodeRabbit

* **Bug Fixes**
* Improved error reporting for missing Git-related resources by
including contextual details.

* **Refactor**
* Reorganized Git routing internals into a new layered implementation to
simplify flow and improve reliability.

* **Tests**
* Updated tests to align with the restructured Git routing
implementation.

<sub>✏️ Tip: You can customize this high-level summary in your review
settings.</sub>
<!-- end of auto-generated comment: release notes by coderabbit.ai -->

(cherry picked from commit cc1ea71)
…it-bug-fix

fix: add code-split for fixing package version change bug
chore: 02/03/26 - Daily Promotion
## Description
Adds SSH key management support to the CE git connect flow.

### Changes
- **Client – ConnectInitialize**: Added `formData.sshKeySource` and
`formData.sshKeyId` to the `useEffect` dependency array so the connect
step reacts to SSH key selection changes.
- **Client – connectSaga**: Passed `sshKeyId` through in
`ConnectRequestParams` so it reaches the backend on connect.
- **Server – GitFSServiceCEImpl**: Fixed reactive chain by replacing
`.flatMap` with `.then` on the `deleteLocalRepo` call during error
handling, ensuring correct signal propagation.

Fixes #`Issue Number`  

## Automation

/ok-to-test tags="@tag.Git"

### 🔍 Cypress test results
<!-- This is an auto-generated comment: Cypress test results  -->
> [!TIP]
> 🟢 🟢 🟢 All cypress tests have passed! 🎉 🎉 🎉
> Workflow run:
<https://github.com/appsmithorg/appsmith/actions/runs/23242272689>
> Commit: fc48d00
> <a
href="https://internal.appsmith.com/app/cypress-dashboard/rundetails-65890b3c81d7400d08fa9ee5?branch=master&workflowId=23242272689&attempt=3"
target="_blank">Cypress dashboard</a>.
> Tags: `@tag.Git`
> Spec:
> <hr>Wed, 18 Mar 2026 12:38:06 UTC
<!-- end of auto-generated comment: Cypress test results  -->

## Test plan
- [ ] Verify git connect flow works with SSH key selection in CE
- [ ] Verify error handling during connect cleans up local repo
correctly
- [ ] Verify existing git connect flow without SSH key still works


Made with [Cursor](https://cursor.com)

<!-- This is an auto-generated comment: release notes by coderabbit.ai
-->
## Summary by CodeRabbit

* **Bug Fixes**
* SSH key selection now reliably triggers UI updates during Git
initialization when the key source or chosen key changes.
* The selected SSH key identifier is correctly included in Git
connection requests so the intended key is used.
* Remote repository error handling improved so analytics complete
reliably and cleanup proceeds without disrupting subsequent steps.
<!-- end of auto-generated comment: release notes by coderabbit.ai -->
…GHSA-g2hc-wmw2-32jr) (#41624)

## Description

Fixes a Broken Object-Level Authorization (BOLA/IDOR) vulnerability in
the application snapshot deletion endpoint (GHSA-g2hc-wmw2-32jr).

**Root cause:** `ApplicationSnapshotServiceCEImpl.deleteSnapshot` was
calling `applicationSnapshotRepository.deleteAllByApplicationId`
directly without performing any authorization check. Any authenticated
user who knew a target application ID could send `DELETE
/api/v1/applications/snapshot/{appId}` and successfully destroy that
application's snapshots — including snapshots belonging to other users
and tenants.

**Fix:** Mirror the existing `restoreSnapshot` pattern in the same
class. Resolve the application via `applicationService.findById(id,
applicationPermission.getEditPermission())` before executing the delete.
When the caller does not have edit permission on the application,
`findById` returns empty, which triggers `switchIfEmpty` and returns
`NO_RESOURCE_FOUND` to the caller — identical behavior to all other
protected operations in this service.

**Changes:**
- `ApplicationSnapshotServiceCEImpl.java`: 5-line change to
`deleteSnapshot` — permission check added, no interface or API signature
changes
- `ApplicationSnapshotServiceTest.java`: Updated existing
`deleteSnapshot` happy-path test to run under a real user + owned
application; added new regression test
`deleteSnapshot_WhenUserHasNoAccess_ThrowsError` asserting that an
inaccessible app ID returns `AppsmithError.NO_RESOURCE_FOUND`

Fixes
https://linear.app/appsmith/issue/APP-15032/high-broken-object-level-authorization-allows-non-owner-to-delete

## Automation

/ok-to-test tags="@tag.All"

### 🔍 Cypress test results
<!-- This is an auto-generated comment: Cypress test results  -->
> [!TIP]
> 🟢 🟢 🟢 All cypress tests have passed! 🎉 🎉 🎉
> Workflow run:
<https://github.com/appsmithorg/appsmith/actions/runs/23285869385>
> Commit: e4911c0
> <a
href="https://internal.appsmith.com/app/cypress-dashboard/rundetails-65890b3c81d7400d08fa9ee5?branch=master&workflowId=23285869385&attempt=2"
target="_blank">Cypress dashboard</a>.
> Tags: `@tag.All`
> Spec:
> <hr>Thu, 19 Mar 2026 09:54:26 UTC
<!-- end of auto-generated comment: Cypress test results  -->

## Communication
Should the DevRel and Marketing teams inform users about this change?
- [ ] Yes
- [x] No

<!-- This is an auto-generated comment: release notes by coderabbit.ai
-->
## Summary by CodeRabbit

* **Bug Fixes**
* Snapshot read and delete actions now validate application existence
and user permissions before proceeding; unauthorized access returns a
clear error.

* **Tests**
* Added tests covering access control for snapshot deletion and snapshot
read-without-data to assert proper error handling for users without
access.
<!-- end of auto-generated comment: release notes by coderabbit.ai -->
## Description
> [!TIP]  
> _Add a TL;DR when the description is longer than 500 words or
extremely technical (helps the content, marketing, and DevRel team)._
>
> _Please also include relevant motivation and context. List any
dependencies that are required for this change. Add links to Notion,
Figma or any other documents that might be relevant to the PR._


Fixes #`Issue Number`  
_or_  
Fixes `Issue URL`
> [!WARNING]  
> _If no issue exists, please create an issue first, and check with the
maintainers if the issue is valid._

## Automation

/ok-to-test tags=""

### 🔍 Cypress test results
<!-- This is an auto-generated comment: Cypress test results  -->
> [!CAUTION]  
> If you modify the content in this section, you are likely to disrupt
the CI result for your PR.

<!-- end of auto-generated comment: Cypress test results  -->


## Communication
Should the DevRel and Marketing teams inform users about this change?
- [ ] Yes
- [ ] No


<!-- This is an auto-generated comment: release notes by coderabbit.ai
-->

## Summary by CodeRabbit

* **Chores**
* Enhanced exception handling framework to preserve error context and
root cause information throughout application layers, improving error
diagnostics and enabling more informative error messages for better
troubleshooting and issue resolution.

<!-- end of auto-generated comment: release notes by coderabbit.ai -->
chore: improve error git route aspect rules (#41650)
Replace long-lived AWS access key/secret pairs in three GitHub Actions
workflows with short-lived credentials via GitHub OIDC and
aws-actions/configure-aws-credentials@v4.

Affected workflows:
- helm-release.yml: S3 Helm chart publish (new secret: HELM_AWS_ROLE_ARN)
- cleanup-dp.yml: EKS deploy preview cleanup
- on-demand-build-docker-image-deploy-preview.yml: EKS deploy preview deploy

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@mohanarpit mohanarpit requested a review from a team as a code owner April 4, 2026 05:22
@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai bot commented Apr 4, 2026

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: f0a2e594-6ee0-41ee-9049-7a2afc151f41

📥 Commits

Reviewing files that changed from the base of the PR and between 7f10885 and 02d9251.

📒 Files selected for processing (11)
  • .github/workflows/cleanup-dp.yml
  • .github/workflows/helm-release.yml
  • .github/workflows/on-demand-build-docker-image-deploy-preview.yml
  • app/client/src/git/components/ConnectModal/ConnectInitialize/index.tsx
  • app/client/src/git/sagas/connectSaga.ts
  • app/server/appsmith-interfaces/src/main/java/com/appsmith/external/exceptions/BaseException.java
  • app/server/appsmith-server/src/main/java/com/appsmith/server/aspect/ce/GitRouteAspectCE.java
  • app/server/appsmith-server/src/main/java/com/appsmith/server/exceptions/AppsmithException.java
  • app/server/appsmith-server/src/main/java/com/appsmith/server/git/fs/GitFSServiceCEImpl.java
  • app/server/appsmith-server/src/main/java/com/appsmith/server/services/ce/ApplicationSnapshotServiceCEImpl.java
  • app/server/appsmith-server/src/test/java/com/appsmith/server/services/ApplicationSnapshotServiceTest.java

Walkthrough

This PR transitions AWS credentials from static environment variables to OIDC role assumption across three CI/CD workflows, extends Git SSH key handling with updated callback dependencies and payload parameters, strengthens exception chaining to preserve stack traces, refactors repository cleanup sequencing, and adds permission validation to snapshot service operations.

Changes

Cohort / File(s) Summary
GitHub Actions Security Hardening
.github/workflows/cleanup-dp.yml, .github/workflows/helm-release.yml, .github/workflows/on-demand-build-docker-image-deploy-preview.yml
Added explicit job-level permissions (id-token: write, contents: read) and introduced aws-actions/configure-aws-credentials@v4 step to assume IAM roles from secrets, replacing static AWS_ACCESS_KEY_ID and AWS_SECRET_ACCESS_KEY environment variables across all three workflows.
Git SSH Key Handling
app/client/src/git/components/ConnectModal/ConnectInitialize/index.tsx, app/client/src/git/sagas/connectSaga.ts
Extended handleNextStep callback dependencies to include sshKeySource and sshKeyId for proper synchronization; added sshKeyId parameter forwarding in connectRequest call.
Exception Chaining & Stack Trace Preservation
app/server/appsmith-interfaces/.../BaseException.java, app/server/appsmith-server/.../AppsmithException.java, app/server/appsmith-server/.../GitRouteAspectCE.java
Added overloaded constructors to BaseException and AppsmithException accepting Throwable cause parameter; updated GitRouteAspectCE error handling to preserve original exception cause during wrapping.
Repository & Snapshot Service Refactoring
app/server/appsmith-server/.../GitFSServiceCEImpl.java, app/server/appsmith-server/.../ApplicationSnapshotServiceCEImpl.java, app/server/appsmith-server/src/test/.../ApplicationSnapshotServiceTest.java
Fixed error-handling chain sequencing in fetchRemoteRepository; added permission-aware application resolution in getWithoutDataByBranchedApplicationId and deleteSnapshot methods; added test coverage for access denial scenarios.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Poem

🔐 Static keys fade to OIDC's embrace,
AWS roles now assume their place,
SSH keys sync with callback care,
Exceptions chain with stack traces fair,
Permissions checked, snapshots secure—
Our code flows cleaner, governance pure! ✨

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 5.88% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (2 passed)
Check name Status Explanation
Title check ✅ Passed The title 'chore(ci): replace AWS static credentials with GitHub OIDC' accurately and concisely summarizes the main change—replacing static AWS credentials with OIDC in CI workflows.
Description check ✅ Passed The PR description includes a clear summary, AWS setup requirements, and a test plan. However, it's missing the required 'Fixes #Issue' link and the 'Communication' section from the template.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch chore/replace-aws-static-creds-with-oidc

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.