Add audit script for ECS short ARNs#1
Open
derek-palmer wants to merge 1 commit into
Open
Conversation
📝 WalkthroughWalkthroughA new Bash script Changes
Sequence Diagram(s)sequenceDiagram
actor User
participant Script as audit-ecs-short-arns.sh
participant Repo as Repository
participant AWS as AWS STS
participant ECS as AWS ECS API
participant IAM as AWS IAM API
User->>Script: Execute with --cluster, --region, etc.
rect rgba(0, 100, 150, 0.5)
Note over Script,Repo: Phase 1: Repository Scan
Script->>Repo: ripgrep for arn:aws:ecs:*:service/*
Repo-->>Script: Matching references
end
rect rgba(0, 150, 100, 0.5)
Note over Script,AWS: Phase 2: AWS Verification
Script->>AWS: get-caller-identity
AWS-->>Script: Account ID validation
end
rect rgba(100, 150, 0, 0.5)
Note over Script,ECS: Phase 3: ECS Service Validation
Script->>ECS: list-services (or use --service)
ECS-->>Script: Service names
Script->>ECS: describe-services (batches of 10)
ECS-->>Script: Service ARN shapes
Script->>Script: Validate ARN format patterns
end
rect rgba(150, 100, 0, 0.5)
Note over Script,IAM: Phase 4 & 5: IAM Policy Scanning
Script->>IAM: list-policies (customer-managed)
IAM-->>Script: Policy list
Script->>IAM: get-policy-version
IAM-->>Script: Policy documents
Script->>IAM: list-roles
IAM-->>Script: Role list
Script->>IAM: get-role-policy
IAM-->>Script: Inline policy documents
Script->>Script: Search for short-form ARN patterns
end
Script-->>User: Pass/Fail with audit results
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
✨ Simplify code
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
audit-ecs-short-arns.sh (2)
14-14: Declare and assign separately to avoid masking return values.Per SC2155: if the subshell fails, the exit code is masked by the
readonlydeclaration. Split declaration and assignment for proper error handling.Proposed fix
-readonly REPO_ROOT="$(cd "$(dirname "${BASH_SOURCE[0]}")/.." && pwd)" +REPO_ROOT="$(cd "$(dirname "${BASH_SOURCE[0]}")/.." && pwd)" +readonly REPO_ROOT🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@audit-ecs-short-arns.sh` at line 14, Split the readonly declaration and the subshell assignment to avoid masking the subshell exit status: first declare the variable name (REPO_ROOT) or assign it normally, then perform the subshell expansion using "$(cd "$(dirname "${BASH_SOURCE[0]}")/.." && pwd)" and after successful assignment mark REPO_ROOT readonly; reference the REPO_ROOT variable and the use of "${BASH_SOURCE[0]}"/.. subshell in the change so reviewers can locate and verify the fix.
184-184: Consider usingjqfor JSON parsing.Parsing JSON with
awk -F'"'is fragile and depends on specific formatting. Ifjqis available (common in AWS environments), it's more robust:Proposed fix
- actual_aws_account_id="$(awk -F'"' '/"Account"/{print $4}' <<< "$caller_identity")" + actual_aws_account_id="$(jq -r '.Account' <<< "$caller_identity")"Add
jqtorequire_commandinmain()if adopted.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@audit-ecs-short-arns.sh` at line 184, Replace the fragile awk JSON parsing that assigns actual_aws_account_id from caller_identity with a jq-based extraction: read the caller_identity JSON and extract the Account field using jq's raw output option, then assign that to actual_aws_account_id; also add jq to the require_command list in main() so the script verifies jq is available before use. Ensure you update the exact assignment line that references actual_aws_account_id and the require_command array in main().
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@audit-ecs-short-arns.sh`:
- Around line 240-254: The script currently calls aws ecs describe-services
twice per batch; change it to call describe-services once, capture the JSON
output into a variable (e.g., services_json) using the same parameters (region,
cluster, --services "${batch[@]}"), then use that captured JSON to both render
the human-readable table and extract ARNs (e.g., use jq to format a table of
serviceArn, desiredCount, serviceName and to produce a whitespace-separated list
for batch_arns). Update references to batch_arns to use the parsed output from
services_json and remove the duplicate aws ecs describe-services invocation.
---
Nitpick comments:
In `@audit-ecs-short-arns.sh`:
- Line 14: Split the readonly declaration and the subshell assignment to avoid
masking the subshell exit status: first declare the variable name (REPO_ROOT) or
assign it normally, then perform the subshell expansion using "$(cd "$(dirname
"${BASH_SOURCE[0]}")/.." && pwd)" and after successful assignment mark REPO_ROOT
readonly; reference the REPO_ROOT variable and the use of "${BASH_SOURCE[0]}"/..
subshell in the change so reviewers can locate and verify the fix.
- Line 184: Replace the fragile awk JSON parsing that assigns
actual_aws_account_id from caller_identity with a jq-based extraction: read the
caller_identity JSON and extract the Account field using jq's raw output option,
then assign that to actual_aws_account_id; also add jq to the require_command
list in main() so the script verifies jq is available before use. Ensure you
update the exact assignment line that references actual_aws_account_id and the
require_command array in main().
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 62434334-73aa-4b4b-9d7c-4f22eb46a4b9
📒 Files selected for processing (1)
audit-ecs-short-arns.sh
Comment on lines
+240
to
+254
| aws ecs describe-services \ | ||
| --region "$AWS_REGION" \ | ||
| --cluster "$ECS_CLUSTER_NAME" \ | ||
| --services "${batch[@]}" \ | ||
| --query 'services[].{arn:serviceArn,desired:desiredCount,name:serviceName}' \ | ||
| --output table | ||
|
|
||
| batch_arns="$( | ||
| aws ecs describe-services \ | ||
| --region "$AWS_REGION" \ | ||
| --cluster "$ECS_CLUSTER_NAME" \ | ||
| --services "${batch[@]}" \ | ||
| --query 'services[].serviceArn' \ | ||
| --output text | ||
| )" |
There was a problem hiding this comment.
Duplicate AWS API calls in each batch iteration.
The aws ecs describe-services command is called twice per batch: once for table display and once to extract ARNs. This doubles API calls, increases execution time, and risks throttling.
Proposed fix - single API call with JSON parsing
for (( i = 0; i < ${`#ECS_SERVICES`[@]}; i += batch_size )); do
batch=("${ECS_SERVICES[@]:$i:$batch_size}")
- aws ecs describe-services \
- --region "$AWS_REGION" \
- --cluster "$ECS_CLUSTER_NAME" \
- --services "${batch[@]}" \
- --query 'services[].{arn:serviceArn,desired:desiredCount,name:serviceName}' \
- --output table
-
- batch_arns="$(
- aws ecs describe-services \
- --region "$AWS_REGION" \
- --cluster "$ECS_CLUSTER_NAME" \
- --services "${batch[@]}" \
- --query 'services[].serviceArn' \
- --output text
- )"
+ local batch_json
+ batch_json="$(
+ aws ecs describe-services \
+ --region "$AWS_REGION" \
+ --cluster "$ECS_CLUSTER_NAME" \
+ --services "${batch[@]}" \
+ --query 'services[].{arn:serviceArn,desired:desiredCount,name:serviceName}' \
+ --output json
+ )"
+
+ # Display table format
+ printf '%s\n' "$batch_json" | jq -r '["ARN","DESIRED","NAME"], (.[] | [.arn, .desired, .name]) | `@tsv`' | column -t
+
+ batch_arns="$(printf '%s\n' "$batch_json" | jq -r '.[].arn')"📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| aws ecs describe-services \ | |
| --region "$AWS_REGION" \ | |
| --cluster "$ECS_CLUSTER_NAME" \ | |
| --services "${batch[@]}" \ | |
| --query 'services[].{arn:serviceArn,desired:desiredCount,name:serviceName}' \ | |
| --output table | |
| batch_arns="$( | |
| aws ecs describe-services \ | |
| --region "$AWS_REGION" \ | |
| --cluster "$ECS_CLUSTER_NAME" \ | |
| --services "${batch[@]}" \ | |
| --query 'services[].serviceArn' \ | |
| --output text | |
| )" | |
| local batch_json | |
| batch_json="$( | |
| aws ecs describe-services \ | |
| --region "$AWS_REGION" \ | |
| --cluster "$ECS_CLUSTER_NAME" \ | |
| --services "${batch[@]}" \ | |
| --query 'services[].{arn:serviceArn,desired:desiredCount,name:serviceName}' \ | |
| --output json | |
| )" | |
| # Display table format | |
| printf '%s\n' "$batch_json" | jq -r '["ARN","DESIRED","NAME"], (.[] | [.arn, .desired, .name]) | `@tsv`' | column -t | |
| batch_arns="$(printf '%s\n' "$batch_json" | jq -r '.[].arn')" |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@audit-ecs-short-arns.sh` around lines 240 - 254, The script currently calls
aws ecs describe-services twice per batch; change it to call describe-services
once, capture the JSON output into a variable (e.g., services_json) using the
same parameters (region, cluster, --services "${batch[@]}"), then use that
captured JSON to both render the human-readable table and extract ARNs (e.g.,
use jq to format a table of serviceArn, desiredCount, serviceName and to produce
a whitespace-separated list for batch_arns). Update references to batch_arns to
use the parsed output from services_json and remove the duplicate aws ecs
describe-services invocation.
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
1 participant
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 by CodeRabbit