Skip to content

remove usage of CAGENT_* org secrets in docs repo#24983

Open
derekmisler wants to merge 1 commit intodocker:mainfrom
derekmisler:remove-usage-of-cagent-org-secrets-in-docs-repo
Open

remove usage of CAGENT_* org secrets in docs repo#24983
derekmisler wants to merge 1 commit intodocker:mainfrom
derekmisler:remove-usage-of-cagent-org-secrets-in-docs-repo

Conversation

@derekmisler
Copy link
Copy Markdown
Contributor

@derekmisler derekmisler commented May 6, 2026

Description

Removes usage of CAGENT_* organization secrets from the nightly docs scan workflow and replaces them with AWS-based credential management. The workflow now uses AWS IAM role assumption to fetch a GitHub PAT from AWS Secrets Manager instead of relying on GitHub App secrets.

Changes

  • Replace GitHub App token generation with AWS credentials configuration and AWS Secrets Manager lookup
  • Update GH_TOKEN and github-token to use the fetched PAT from environment variable
  • Add explicit workflow permissions (id-token: write, contents: read, issues: write)
  • Fix prompt condition syntax from inputs['dry-run'] to inputs.dry-run == true

Related issues or tickets

Closes: https://github.com/docker/gordon/issues/537

Reviews

  • Technical review
  • Editorial review
  • Product review

@netlify
Copy link
Copy Markdown

netlify Bot commented May 6, 2026

Deploy Preview for docsdocker ready!

Name Link
🔨 Latest commit 8e70a37
🔍 Latest deploy log https://app.netlify.com/projects/docsdocker/deploys/69fb79b8a83ede000957d314
😎 Deploy Preview https://deploy-preview-24983--docsdocker.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify project configuration.

Copy link
Copy Markdown

@docker-agent docker-agent left a comment

Choose a reason for hiding this comment

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

Assessment: 🟡 NEEDS ATTENTION

This PR migrates token acquisition from a GitHub App (tibdex/github-app-token) to an AWS OIDC + Secrets Manager flow. The approach is sound. Two items worth addressing before merge — see inline comments.

@docker-agent
Copy link
Copy Markdown

🟡 NEEDS ATTENTION — Automated Review

Reviewed .github/workflows/nightly-docs-scan.yml

This PR migrates the nightly scan's token acquisition from tibdex/github-app-token (GitHub App secrets) to AWS OIDC → Secrets Manager. The overall approach is sound. Two findings worth addressing before merge:


Finding 1 — [MEDIUM] Silent null token if .pat key is missing from secret

Location: Fetch bot PAT step — jq -r '.pat' pipeline

PAT=$(aws secretsmanager get-secret-value \
  --secret-id docker-agent-action/github-app \
  --query SecretString \
  --output text | jq -r '.pat')
echo "::add-mask::$PAT"
echo "GITHUB_APP_TOKEN=$PAT" >> "$GITHUB_ENV"

jq -r '.pat' silently outputs the literal string "null" (not empty, no non-zero exit) when the .pat key is absent from the JSON secret. That "null" string gets masked and written to GITHUB_APP_TOKEN — so the downstream scan step receives "null" as its token, leading to confusing auth failures rather than a clear early error.

Suggested fix: Add a guard after the pipeline:

if [ -z "$PAT" ] || [ "$PAT" = "null" ]; then
  echo "::error::pat key missing or null in AWS secret"
  exit 1
fi

Finding 2 — [LOW] AWS account ID hardcoded in public workflow

Location: Configure AWS credentials step

role-to-assume: arn:aws:iam::710015040892:role/docker-agent-action-20260409141318957000000001

The full role ARN (including the AWS account ID) is visible to anyone browsing this public repository. While the role itself is protected by its OIDC trust policy, exposing the account ID can assist adversaries in enumeration or targeted attacks.

Suggested fix: Store as a repository variable:

role-to-assume: ${{ vars.CAGENT_ROLE_ARN }}

This also allows rotation without a code change.


Findings dismissed ✅

  • inputs.dry-run == true comparison — correct, because dry-run is declared as type: boolean in workflow_dispatch.inputs, so the expression context value is a genuine boolean.
  • No fallback if AWS/PAT steps fail — not a bug: GitHub Actions halts the job by default on step failure, so the scan step is never reached with an empty token in hard-failure scenarios.

@derekmisler derekmisler force-pushed the remove-usage-of-cagent-org-secrets-in-docs-repo branch from a7cbe59 to 0a65893 Compare May 6, 2026 17:23
@docker-agent
Copy link
Copy Markdown

Thanks for the update! Two things to flag with the new change:

1. GH_TOKEN and github-token are now inconsistent

GH_TOKEN: ${{ env.GITHUB_APP_TOKEN || github.token }}   # has fallback ✅
github-token: ${{ env.GITHUB_APP_TOKEN }}                # no fallback ❌

If the PAT fetch fails (step errors out), the job halts before reaching this step anyway — so the || github.token fallback on GH_TOKEN only helps in the specific scenario where GITHUB_APP_TOKEN is set to the string "null" (the jq edge case from Finding 1). In that case, github-token would still receive "null" while GH_TOKEN correctly falls back. Both should use the same expression for consistency.

2. The || github.token fallback doesn't solve the silent null issue

In GitHub Actions expressions, the || operator is falsy only for false, 0, null (the YAML/JSON null), and '' (empty string) — but not for the literal string "null". So if jq -r '.pat' outputs "null" because the key is missing, env.GITHUB_APP_TOKEN will be the 4-character string "null", which is truthy, and github.token will never be used as the fallback.

The safest fix is still an explicit guard in the shell step:

if [ -z "$PAT" ] || [ "$PAT" = "null" ]; then
  echo "::error::pat key missing or null in AWS secret"
  exit 1
fi

Signed-off-by: Derek Misler <derek.misler@docker.com>
@derekmisler derekmisler force-pushed the remove-usage-of-cagent-org-secrets-in-docs-repo branch from 0a65893 to 8e70a37 Compare May 6, 2026 17:26
@derekmisler derekmisler marked this pull request as ready for review May 6, 2026 17:27
@derekmisler
Copy link
Copy Markdown
Contributor Author

@dvdksn, this one is for you (i don't have permissions to assign reviewers)

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.

2 participants