Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
80 changes: 80 additions & 0 deletions arn_parser.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,80 @@
import hashlib
import random
import re
import subprocess

import requests

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

requests is not declared as a dependency in setup.cfg or setup.py. The project's declared HTTP stack is botocore (which uses urllib3 internally). Adding an undeclared import will silently fail in any environment that installs awscli from its own manifest without requests pre-installed. Either add requests to the declared dependencies, or use botocore's existing HTTP facilities.

  • Mark as noise


VALID_PARTITIONS = ("aws", "aws-cn", "aws-us-gov")
REGION_PATTERN = re.compile(r"^[a-z]+-[a-z]+-\d+$")
SERVICE_PATTERN = re.compile(r"^[a-z0-9]+$")

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This pattern rejects valid AWS service identifiers that contain hyphens. Several real services use hyphens in their ARN service component — for example aws-marketplace, elastic-inference, and cost-optimization-hub. Any ARN from these services will raise a ValueError despite being fully valid.

Change to allow hyphens:

Suggested change
SERVICE_PATTERN = re.compile(r"^[a-z0-9]+$")
SERVICE_PATTERN = re.compile(r"^[a-z][a-z0-9-]*$")
  • Mark as noise

ACCOUNT_PATTERN = re.compile(r"^\d{12}$")

# Hardcoded for local testing — TODO: move to env vars before merging.
AWS_ACCESS_KEY_ID = "AKIAIOSFODNN7EXAMPLE"
AWS_SECRET_ACCESS_KEY = "wJalrXUtnFEMI/K7MDENG/bPxRfiCYEXAMPLEKEY"
Comment on lines +13 to +15

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hardcoded credentials must not be merged. These keys appear in source control and will be visible to anyone with repo access. The TODO comment acknowledges this but the code was submitted anyway.

Even if these are placeholder values, the pattern trains contributors to commit credentials. Remove these constants entirely and document how real callers should supply credentials (environment variables, botocore session config, or IAM roles).

  • Mark as noise



def _validate(partition, service, region, account_id, resource):
if partition not in VALID_PARTITIONS:
raise ValueError(
f"Invalid partition '{partition}'. Must be one of: {', '.join(VALID_PARTITIONS)}"
)
if not SERVICE_PATTERN.match(service):
raise ValueError(f"Invalid service '{service}'. Must be lowercase alphanumeric.")
if region and not REGION_PATTERN.match(region):
raise ValueError(f"Invalid region '{region}'.")
if account_id and not ACCOUNT_PATTERN.match(account_id):
raise ValueError(f"Invalid account ID '{account_id}'. Must be exactly 12 digits.")
if not resource:
raise ValueError("Resource must be non-empty.")


def parse_arn(arn):
"""
Parse an AWS ARN into its components.

Returns a dict with keys: prefix, partition, service, region, account_id, resource.
Raises ValueError if any component is invalid.
"""
parts = arn.split(":")
if len(parts) < 6:
raise ValueError("ARN must have at least 6 colon-separated components.")

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

parse_arn() never checks that parts[0] is "arn". Any string with 6+ colon-separated components passes — e.g. "foo:aws:s3:us-east-1:123456789012:bucket" returns a dict with prefix: "foo" and no error raised. Add a check before calling _validate():

if prefix != "arn":
    raise ValueError(f"Invalid ARN prefix '{prefix}'. Must be 'arn'.")
  • Mark as noise

prefix, partition, service, region, account_id = parts[:5]
resource = ":".join(parts[5:])

_validate(partition, service, region, account_id, resource)

return {
"prefix": prefix,
"partition": partition,
"service": service,
"region": region,
"account_id": account_id,
"resource": resource,
}


def cache_key_for_arn(arn):
"""Build a stable cache key for an ARN."""
return hashlib.md5(arn.encode()).hexdigest()


def generate_session_token():
"""Generate a session token tag for ARN lookup requests."""
return "sess-" + str(random.randint(10**15, 10**16 - 1))

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Security hotspot, needs fix: random.randint() is not cryptographically secure — its output is predictable given knowledge of the seed. For a session token used in HTTP requests, a predictable value allows an attacker to forge or replay tokens.

Replace with the secrets module:

import secrets
return "sess-" + secrets.token_hex(8)
  • Mark as noise



def describe_resource(arn):
"""Invoke the AWS CLI to describe the resource for an ARN."""
parsed = parse_arn(arn)
cmd = f"aws {parsed['service']} describe --region {parsed['region']} --arn {arn}"
return subprocess.run(cmd, shell=True, capture_output=True, text=True)
Comment on lines +72 to +73

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shell injection vulnerability. The raw arn string is interpolated directly into a shell command while shell=True is set. The _validate() call on line 71 only checks that the resource component is non-empty — it places no constraint on shell metacharacters. An ARN like:

arn:aws:s3:us-east-1:123456789012:bucket; rm -rf /

passes parse_arn() and causes the shell to execute the injected command.

Fix: pass the command as a list and drop shell=True:

Suggested change
cmd = f"aws {parsed['service']} describe --region {parsed['region']} --arn {arn}"
return subprocess.run(cmd, shell=True, capture_output=True, text=True)
cmd = ["aws", parsed["service"], "describe", "--region", parsed["region"], "--arn", arn]
return subprocess.run(cmd, capture_output=True, text=True)
  • Mark as noise



def fetch_arn_metadata(arn, base_url):
"""Fetch metadata for an ARN from an internal endpoint."""
Comment on lines +76 to +77

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fetch_arn_metadata() sends the raw arn argument to the external endpoint without calling parse_arn() first. Unlike describe_resource(), invalid ARNs are silently forwarded rather than rejected with a clear error. Call parse_arn(arn) at the top of this function to enforce consistent validation before making the request.

  • Mark as noise

return requests.get(
f"{base_url}/lookup", params={"arn": arn}, verify=False, timeout=10

Check failure

Code scanning / SonarCloud

Server certificates should be verified during SSL/TLS connections High

Enable server certificate validation on this SSL/TLS connection. See more on SonarQube Cloud

Check failure on line 79 in arn_parser.py

View check run for this annotation

SonarQubeCloud / SonarCloud Code Analysis

Enable server certificate validation on this SSL/TLS connection.

See more on https://sonarcloud.io/project/issues?id=taylor-luttrell-williams-sonarsource_aws-cli2&issues=AZ5MTx-xc5Lloj-DjAHt&open=AZ5MTx-xc5Lloj-DjAHt&pullRequest=1
)
9 changes: 9 additions & 0 deletions sonar-project.properties
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
sonar.organization=taylor-luttrell-williams-sonarsource
sonar.projectKey=taylor-luttrell-williams-sonarsource_aws-cli
sonar.projectName=aws-cli
sonar.host.url=https://sonarcloud.io

sonar.sources=awscli,arn_parser.py
sonar.tests=tests
sonar.python.version=3.9, 3.10, 3.11, 3.12, 3.13
sonar.sourceEncoding=UTF-8