Skip to content

[security-audit-agent] Security Audit Findings — 2026-06-01#69

Closed
rosa-regional-platform-ci wants to merge 1 commit into
openshift-online:mainfrom
rosa-regional-platform-ci:security-audit-agent/findings-2026-06-01
Closed

[security-audit-agent] Security Audit Findings — 2026-06-01#69
rosa-regional-platform-ci wants to merge 1 commit into
openshift-online:mainfrom
rosa-regional-platform-ci:security-audit-agent/findings-2026-06-01

Conversation

@rosa-regional-platform-ci

Copy link
Copy Markdown

Security Audit — rosa-regional-platform-cli

Date: 2026-06-01
Auditor: Automated security-audit-agent
Method: Full static analysis of all Go source files, Lambda handler, CLI commands, test infrastructure
Action required: Triage each finding. This PR does not contain fixes — it communicates security issues for owner review.


HIGH Findings

HIGH-1 — Lambda Handler Logs Full Event Payload to CloudWatch

File: internal/lambda/handler.go line 38
Risk: Infrastructure topology, cluster names, and OIDC issuer URLs exposed in CloudWatch logs

fmt.Printf("Received event: %+v\n", event)

The Lambda handler prints the complete incoming event struct to stdout on every invocation. Lambda stdout is sent to CloudWatch Logs. The event contains sensitive infrastructure details: cluster names, OIDC issuer URLs, thumbprints, and potentially IAM role ARNs. CloudWatch Logs are often accessible to a broader set of principals than the Lambda function itself, and log data persists based on the retention policy.

Attack vector:

  1. Attacker gains logs:GetLogEvents permission on the Lambda log group (e.g., via compromised developer credentials, overly permissive IAM role, or accidental CloudWatch public access).
  2. Attacker reads full event payloads from every Lambda invocation.
  3. Attacker extracts OIDC issuer URLs, cluster IDs, and infrastructure topology for targeted reconnaissance.
  4. OIDC thumbprints + issuer URLs allow the attacker to set up a rogue OIDC provider if they can intercept or manipulate the certificate chain.

Mitigation:

  • Remove the full event log line. If debugging is needed, log only non-sensitive fields:
    log.Printf("Received event for cluster: %s", event.ClusterID)
  • Apply log scrubbing middleware for the Lambda handler.
  • Restrict the Lambda CloudWatch log group with a resource policy limiting read access to specific authorized principals.

HIGH-2 — SHA-1 Used for OIDC Provider Thumbprint Calculation

File: internal/crypto/thumbprint.go lines 1–54
Risk: Cryptographic weakness in OIDC trust anchor calculation

import "crypto/sha1"
hash := sha1.Sum(rootCert.Raw)
return hex.EncodeToString(hash[:]), nil

The OIDC provider thumbprint is computed using SHA-1, which is cryptographically broken for collision resistance. While AWS IAM currently requires SHA-1 thumbprints for OIDC provider registration (a legacy AWS limitation), this creates a dependency on SHA-1's continued relative security for the integrity of the OIDC trust chain.

Attack vector: A SHA-1 chosen-prefix collision attack (demonstrated feasible since 2019 with SHAttered and similar research) could allow an attacker to craft a certificate with a SHA-1 thumbprint matching a legitimate OIDC CA. If AWS expands OIDC validation beyond thumbprint matching (e.g., full certificate verification), this dependency becomes more dangerous. More immediately, any tooling that re-verifies thumbprints using SHA-1 is vulnerable.

Mitigation:

  • Document in code comments that SHA-1 is used solely because AWS IAM requires it, not as a design choice, and link to the AWS documentation.
  • File a tracking issue to migrate when AWS supports SHA-256 thumbprints for OIDC providers.
  • Add a comment explicitly noting this is a forced dependency, so future maintainers don't copy the pattern elsewhere.
  • Monitor the AWS changelog for SHA-256 OIDC thumbprint support.

MEDIUM Findings

MED-1 — Cluster Configuration Output Files Written with World-Readable Permissions (0644)

File: internal/commands/cluster/create.go lines 197, 243
Risk: Local information disclosure — other users on the same host can read cluster configuration

if err := os.WriteFile(opts.outputFile, jsonBytes, 0644); err != nil {
    return fmt.Errorf("failed to write output file: %w", err)
}

The --output-file and --dry-run flags write cluster configuration JSON to disk with permission 0644 (owner read/write, group read, others read). The configuration contains IAM role ARNs, VPC IDs, subnet IDs, and cluster metadata — information that enables targeted reconnaissance.

Note the inconsistency: the API URL config file in internal/config/config.go line 92 correctly uses 0600. The cluster output file should match this pattern.

Attack vector: On a shared bastion host or developer workstation, another local user (or a compromised process running as a different user) reads the world-readable output file and extracts VPC topology and IAM role ARNs.

Mitigation: Change to 0600:

if err := os.WriteFile(opts.outputFile, jsonBytes, 0600); err != nil {

MED-2 — URL Query Parameters Not Properly Encoded (Potential Injection)

File: internal/commands/cluster/list.go lines 95–98
Risk: Malformed query strings if status value contains special characters; potential API-level injection

endpoint := fmt.Sprintf("%s/api/v0/clusters?limit=%d&offset=%d", baseURL, opts.limit, opts.offset)
if opts.status != "" {
    endpoint = fmt.Sprintf("%s&status=%s", endpoint, opts.status)  // no URL encoding
}

The status query parameter is appended without url.QueryEscape() or url.Values.Encode(). If the status value contains &, =, #, or other URL-special characters (e.g., from shell globbing, tab completion, or an attacker providing a crafted value), the resulting URL is malformed or injects additional query parameters.

Attack vector: User passes --status "active&admin=true". The API receives ?status=active&admin=true, potentially matching a query parameter the API interprets differently. Depending on API routing logic, this could bypass filters or trigger unintended behavior.

Mitigation:

params := url.Values{}
params.Set("limit", strconv.Itoa(opts.limit))
params.Set("offset", strconv.Itoa(opts.offset))
if opts.status != "" {
    params.Set("status", opts.status)
}
endpoint := fmt.Sprintf("%s/api/v0/clusters?%s", baseURL, params.Encode())

MED-3 — API Error Responses Printed Verbatim to User Output

Files:

  • internal/services/cluster/service.go line 257
  • internal/commands/cluster/list.go line 152
    Risk: Server-side error messages containing internal details exposed to users and logged
return nil, fmt.Errorf("API request failed with status %d: %s", resp.StatusCode, string(body))

The full API response body is included in error messages. If the server returns stack traces, internal endpoint names, database errors, or configuration details in error responses, these are surfaced directly to the CLI user and written to any CLI output logs.

Mitigation:

  • Parse the error response body and extract only the user-facing message field (e.g., error.message).
  • Log the full response body at debug level internally, but surface only a sanitized message.

MED-4 — SSRF Risk in Login Command — URL Not Restricted to External Hosts

File: internal/commands/login/login.go lines 40–66
Risk: CLI can be directed to send authentication requests to internal AWS metadata service or cluster-internal endpoints

if parsedURL.Scheme != "http" && parsedURL.Scheme != "https" {
    return fmt.Errorf("URL scheme must be http or https")
}

The URL validation only checks the scheme. It does not block:

  • http://169.254.169.254/ (AWS EC2 metadata service)
  • http://127.0.0.1:8080/ (local services)
  • http://10.0.0.1/ (VPC-internal services)

Attack vector:

  1. Attacker tricks a developer or CI system into running rosactl login --url http://169.254.169.254/latest/api/token.
  2. The CLI sends an HTTP request to the metadata service.
  3. Response (possibly including instance credentials) is handled/logged by the CLI.

Mitigation:

  • Block loopback, link-local (169.254.x.x), and RFC-1918 addresses in URL validation.
  • Consider maintaining an explicit allowlist of known API base URL patterns (e.g., *.rosa.aws.amazon.com).

LOW Findings

LOW-1 — Config Directory Created with World-Readable Permissions (0755)

File: internal/config/config.go line 44
Risk: Other local users can enumerate the existence of the .rosactl/ config directory

if err := os.MkdirAll(configDirPath, 0755); err != nil {

While the config file is correctly created with 0600, the directory is world-readable and world-executable (0755). On a shared system, any user can list the directory contents, see that the config file exists, and determine that rosactl is installed and configured.

Mitigation: Use 0700 for the config directory:

if err := os.MkdirAll(configDirPath, 0700); err != nil {

LOW-2 — Hardcoded Test Credentials in Test Infrastructure

File: test/localstack/lambda_test.go lines 65, 413–414
Risk: Establishes a pattern of hardcoded credentials in the codebase; accidental promotion to production

config.WithCredentialsProvider(credentials.NewStaticCredentialsProvider("test", "test", "")),
"AWS_ACCESS_KEY_ID=test",
"AWS_SECRET_ACCESS_KEY=test",

These are dummy LocalStack credentials and not real AWS credentials. However, the pattern of hardcoding credential values in source files — even dummy ones — normalizes the practice and increases the risk that a future developer will hardcode a real credential.

Mitigation: Source test credentials from environment variables with documented defaults for local development:

accessKey := os.Getenv("TEST_AWS_ACCESS_KEY_ID")
if accessKey == "" { accessKey = "test" }

LOW-3 — No Audit Logging for Security-Critical CLI Operations

Files: internal/commands/cluster/create.go, internal/commands/cluster/delete.go
Risk: No local or remote audit trail for cluster lifecycle operations

The CLI does not log security-critical operations (cluster creation, deletion, IAM role attachment) to a local audit log or structured output. In a shared environment or CI/CD context, this makes it impossible to reconstruct who ran which command, when, and with what parameters.

Mitigation: Write structured JSON audit records to ~/.rosactl/audit.log (with 0600 permissions) for all mutation operations. Include: timestamp, operation, user identity (from AWS STS caller identity), cluster ID, and exit status.


LOW-4 — Environment Variables for AWS Region/Profile Not Validated

File: internal/aws/session.go lines 14–20
Risk: Attacker with environment control can redirect AWS API calls to unintended regions

if region := os.Getenv("ROSACTL_REGION"); region != "" {
    opts = append(opts, config.WithRegion(region))
}

No validation that the region is a valid AWS region string. An attacker who controls the environment (e.g., via a malicious Makefile include, compromised shell profile, or container environment injection) could set ROSACTL_REGION to a value that causes the AWS SDK to behave unexpectedly.

Mitigation: Validate the region format matches the AWS region pattern (^[a-z]{2}-[a-z]+-\d$) before passing it to the SDK.


This audit was generated by the security-audit-agent on 2026-06-01. Findings reflect the state of the repository at the time of analysis.

@openshift-ci

openshift-ci Bot commented Jun 1, 2026

Copy link
Copy Markdown

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: rosa-regional-platform-ci
Once this PR has been reviewed and has the lgtm label, please assign iamkirkbater for approval. For more information see the Code Review Process.

The full list of commands accepted by this bot can be found here.

Details Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@coderabbitai

coderabbitai Bot commented Jun 1, 2026

Copy link
Copy Markdown

Warning

Review limit reached

@rosa-regional-platform-ci, we couldn't start this review because you've reached your PR review rate limit.

More reviews will be available in 57 minutes and 46 seconds. Learn how PR review limits work.

Your organization has run out of usage credits. Purchase more in the billing tab.

⌛ How to resolve this issue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

We recommend that you space out your commits to avoid hitting the rate limit.

🚦 How do rate limits work?

CodeRabbit enforces hourly rate limits for each developer per organization.

Our paid plans include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available.

Please see our Fair Usage Limits Policy for further information.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Enterprise

Run ID: 0753863b-ceeb-4f70-b894-ded3a95f6c0e

📥 Commits

Reviewing files that changed from the base of the PR and between 9746f29 and 525f8af.

📒 Files selected for processing (1)
  • SECURITY-AUDIT-2026-06-01.md
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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.

@openshift-ci openshift-ci Bot added the needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. label Jun 1, 2026
@openshift-ci

openshift-ci Bot commented Jun 1, 2026

Copy link
Copy Markdown

Hi @rosa-regional-platform-ci. Thanks for your PR.

I'm waiting for a openshift-online member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work.

Tip

We noticed you've done this a few times! Consider joining the org to skip this step and gain /lgtm and other bot rights. We recommend asking approvers on your previous PRs to sponsor you.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

Details

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository.

@cdoan1

cdoan1 commented Jun 3, 2026

Copy link
Copy Markdown
Collaborator

/ok-to-test

@openshift-ci openshift-ci Bot added ok-to-test Indicates a non-member PR verified by an org member that is safe to test. and removed needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. labels Jun 3, 2026
@rosa-regional-platform-ci

Copy link
Copy Markdown
Author

Superseded by #75 (2026-06-15 consolidated audit).

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

Labels

ok-to-test Indicates a non-member PR verified by an org member that is safe to test.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants