Skip to content

Guard AWS credential steps against pull_request runs - #31

Merged
kbateman merged 1 commit into
mainfrom
fix/guard-aws-creds-on-pr
Sep 21, 2026
Merged

kbateman merged 1 commit into
mainfrom
fix/guard-aws-creds-on-pr

Conversation

@kbateman

Copy link
Copy Markdown
Collaborator

Summary

  • Add if: inputs.publish to the Configure AWS credentials and Login to Amazon ECR steps in both jobs of artifacts.yaml so the OIDC token is not minted and the ECR IAM role is not assumed on PR runs (where publish: false).
  • Add an equivalent if: github.event_name == 'push' && startsWith(github.ref, 'refs/tags/deploy/charts/v') guard to the same two steps in helm.yml so credentials are only obtained when a chart is actually being pushed. This mirrors the existing helm_push_enabled gate later in the workflow.

Motivation

Both workflows trigger on pull_request and declare id-token: write, and the AWS credential + ECR login steps were unconditional. On PR events no image is pushed (docker/build-push-action writes to a local tar when publish is false) and no chart is pushed (helm push only runs on matching tags), so assuming the IAM role on a runner executing PR-controlled code offered no benefit and needlessly exposed short-lived STS credentials and an OIDC JWT.

Test plan

  • CI runs on this PR complete without needing AWS credentials (artifacts job still builds and scans locally; helm job still lints).
  • After merge, a chart tag push (deploy/charts/v*) still assumes the role and pushes the chart.
  • After merge, a main push publishes controller / refresher images via the normal path.

🤖 Generated with Claude Code

Configure AWS credentials and Login to Amazon ECR were executed on every
workflow run, including pull_request events, even though no image or chart
is actually pushed on a PR. This unnecessarily minted an OIDC token and
assumed the ECR IAM role on runners executing PR-controlled code.

Gate both steps on the same condition that controls the actual publish:
inputs.publish for artifacts.yaml, and a tag-push check for helm.yml.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

@dshinde-cbc dshinde-cbc left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Trying my luck

@kbateman
kbateman merged commit e5385ce into main Sep 21, 2026
7 checks passed
@kbateman
kbateman deleted the fix/guard-aws-creds-on-pr branch September 21, 2026 10:13
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.

3 participants