Skip to content

Development#53

Merged
NickSavino merged 3 commits intomainfrom
development
Feb 4, 2026
Merged

Development#53
NickSavino merged 3 commits intomainfrom
development

Conversation

@NickSavino
Copy link
Copy Markdown
Contributor

@NickSavino NickSavino commented Feb 4, 2026

PR Type

Enhancement, Bug fix


Description

  • Add IAM policies for manual EC2/RDS start-stop via GitHub Actions

  • Fix shell script invocations with explicit bash command prefix

  • Correct variable escaping in migration script database URL

  • Remove redundant RDS wait command from stop workflow


Diagram Walkthrough

flowchart LR
  A["GitHub Actions<br/>Workflows"] -->|"Enhanced IAM<br/>Permissions"| B["EC2/RDS<br/>Start-Stop"]
  C["Deployment<br/>Scripts"] -->|"Fixed Shell<br/>Invocation"| D["Improved<br/>Reliability"]
  E["Variable<br/>Escaping"] -->|"Corrected"| F["Migration<br/>Script"]
Loading

File Walkthrough

Relevant files
Enhancement
main.tf
Add GitHub Actions EC2/RDS start-stop IAM policies             

infra/modules/github_oidc/main.tf

  • Added new IAM policy document github_start_stop with EC2 and RDS
    permissions
  • Implemented EC2 start/stop actions with project tag-based conditions
  • Implemented RDS start/stop actions with project tag-based conditions
  • Created policy attachment to deploy role for manual resource control
+68/-0   
stop-env.yml
Remove redundant RDS wait command                                               

.github/workflows/stop-env.yml

  • Removed redundant aws rds wait db-instance-stopped command
  • Simplifies workflow by removing unnecessary wait condition
+1/-2     
Bug fix
deploy_via_ssm.sh
Add explicit bash command to script invocation                     

scripts/ci/deploy_via_ssm.sh

  • Changed script invocation from ./scripts/ci/ssm_run_and_wait.sh to
    bash ./scripts/ci/ssm_run_and_wait.sh
  • Ensures explicit bash shell execution for better compatibility
+1/-1     
migrate_via_ssm.sh
Fix variable escaping and script invocation                           

scripts/ci/migrate_via_ssm.sh

  • Fixed variable escaping in GOOSE_DBSTRING from \\$DBURL to \$DBURL
  • Changed script invocation from ./scripts/ci/ssm_run_and_wait.sh to
    bash ./scripts/ci/ssm_run_and_wait.sh
  • Corrects database URL parameter passing in Docker migration command
+2/-2     

NickSavino and others added 3 commits February 3, 2026 21:23
* fixed migrate_via_ssm.sh script

* Changed line endings to LF in scripts and added bash command for safety

---------

Co-authored-by: NicolaSavino <nick.savino@arcurve.com>
* fixed migrate_via_ssm.sh script

* Changed line endings to LF in scripts and added bash command for safety

---------

Co-authored-by: NicolaSavino <nick.savino@arcurve.com>
@coderabbitai
Copy link
Copy Markdown

coderabbitai bot commented Feb 4, 2026

Important

Review skipped

Auto reviews are disabled on base/target branches other than the default branch.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

  • 🔍 Trigger a full review

Note

🎁 Summarized by CodeRabbit Free

Your organization is on the Free plan. CodeRabbit will generate a high-level summary and a walkthrough for each pull request. For a comprehensive line-by-line review, please upgrade your subscription to CodeRabbit Pro by visiting https://app.coderabbit.ai/login.

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

@NickSavino NickSavino merged commit b63904b into main Feb 4, 2026
1 check passed
@qodo-code-review
Copy link
Copy Markdown

PR Compliance Guide 🔍

Below is a summary of compliance checks for this PR:

Security Compliance
Overbroad IAM policy

Description: The new github_start_stop IAM policy allows EC2/RDS start/stop across all instances/DBs in
any region/account as long as a Project=cgc-2026 tag matches (and also grants Describe* on
*), which could enable unintended resource control/enumeration if an attacker can
influence tags or if tagging governance is weak.
main.tf [118-183]

Referred Code
data "aws_iam_policy_document" "github_start_stop" {

    statement {
      sid = "EC2Describe"
      effect = "Allow"
      actions = [ 
        "ec2:DescribeInstances",
        "ec2:DescribeInstanceStatus",
        "ec2:DescribeTags"
       ]
       resources = ["*"]
    }

    statement {
      sid = "EC2StartStop"
      effect = "Allow"
      actions = [
        "ec2:StartInstances",
        "ec2:StopInstances"
      ]
      resources = ["arn:aws:ec2:*:*:instance/*"]


 ... (clipped 45 lines)
Ticket Compliance
🎫 No ticket provided
  • Create ticket/issue
Codebase Duplication Compliance
Codebase context is not defined

Follow the guide to enable codebase context checks.

Custom Compliance
🟢
Generic: Meaningful Naming and Self-Documenting Code

Objective: Ensure all identifiers clearly express their purpose and intent, making code
self-documenting

Status: Passed

Learn more about managing compliance generic rules or creating your own custom rules

Generic: Robust Error Handling and Edge Case Management

Objective: Ensure comprehensive error handling that provides meaningful context and graceful
degradation

Status: Passed

Learn more about managing compliance generic rules or creating your own custom rules

Generic: Secure Error Handling

Objective: To prevent the leakage of sensitive system information through error messages while
providing sufficient detail for internal debugging.

Status: Passed

Learn more about managing compliance generic rules or creating your own custom rules

Generic: Security-First Input Validation and Data Handling

Objective: Ensure all data inputs are validated, sanitized, and handled securely to prevent
vulnerabilities

Status: Passed

Learn more about managing compliance generic rules or creating your own custom rules

Generic: Comprehensive Audit Trails

Objective: To create a detailed and reliable record of critical system actions for security analysis
and compliance.

Status:
Audit trail unclear: The PR adds/changes operational start/stop actions for EC2/RDS but the diff does not show
any explicit audit logging or confirmation that CloudTrail/SSM command logging captures
actor, timestamp, action, and outcome.

Referred Code
- name: Stop EC2 and RDS
  run: |
    set -euo pipefail

    aws ec2 stop-instances --instance-ids "${{ secrets.EC2_INSTANCE_ID }}"
    aws ec2 wait instance-stopped --instance-ids "${{ secrets.EC2_INSTANCE_ID }}"

    aws rds stop-db-instance --db-instance-identifier "${{ secrets.RDS_INSTANCE_ID }}"

Learn more about managing compliance generic rules or creating your own custom rules

Generic: Secure Logging Practices

Objective: To ensure logs are useful for debugging and auditing without exposing sensitive
information like PII, PHI, or cardholder data.

Status:
Secret exposure risk: The PR passes decrypted DBURL (and in deploy, CLERK_SECRET_KEY) via SSM-executed shell
commands, but the diff does not confirm whether ssm_run_and_wait.sh/SSM command logging
could record these values in logs or outputs.

Referred Code
    "DBURL=\$(aws ssm get-parameter --region $AWS_REGION --with-decryption --name /cgc-2026-prod/api/database_url --query Parameter.Value --output text)",
    "sudo docker pull $MIGRATOR_IMAGE_URI",
    "sudo docker run --rm -e GOOSE_DRIVER=postgres -e GOOSE_DBSTRING=\"\$DBURL\" $MIGRATOR_IMAGE_URI -dir ./db/migrations up"
]
JSON
)

bash ./scripts/ci/ssm_run_and_wait.sh "${COMMANDS_JSON}"

Learn more about managing compliance generic rules or creating your own custom rules

Compliance status legend 🟢 - Fully Compliant
🟡 - Partial Compliant
🔴 - Not Compliant
⚪ - Requires Further Human Verification
🏷️ - Compliance label

@qodo-code-review
Copy link
Copy Markdown

PR Code Suggestions ✨

Explore these optional code suggestions:

CategorySuggestion                                                                                                                                    Impact
Possible issue
Fix RDS tag condition key

In the IAM policy for RDS, change the condition key from the generic
aws:ResourceTag/Project to the service-specific rds:ResourceTag/Project to
correctly match RDS instance tags.

infra/modules/github_oidc/main.tf [157-171]

 statement {
   sid = "RDSStartStop"
   effect = "Allow"
   actions = [
     "rds:StartDBInstance",
     "rds:StopDBInstance"
   ]
   resources = ["arn:aws:rds:*:*:db:*"]
 
   condition {
     test = "StringEquals"
-    variable = "aws:ResourceTag/Project"
+    variable = "rds:ResourceTag/Project"
     values = ["cgc-2026"]
   }
 }
  • Apply / Chat
Suggestion importance[1-10]: 9

__

Why: This suggestion corrects a bug where the IAM policy would fail to authorize RDS actions because it uses an incorrect condition key (aws:ResourceTag/Project) instead of the required service-specific key (rds:ResourceTag/Project).

High
  • More

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants