Skip to content

Fix the SDK release safety check - #119

Merged
aburkard merged 1 commit into
mainfrom
codex/fix-production-release-doctor
Sep 3, 2026
Merged

Fix the SDK release safety check#119
aburkard merged 1 commit into
mainfrom
codex/fix-production-release-doctor

Conversation

@aburkard

@aburkard aburkard commented Sep 3, 2026

Copy link
Copy Markdown
Contributor

The release workflow uses PRODUCTION_REPO_TOKEN, but the release-doctor check still looked for the retired RELEASE_PLEASE_TOKEN name and never passed the current secret into the script. This makes the release PR fail even when the real release token is configured. Align the check with the release workflow and expose the secret only to that validation step.\n\nVerified with bash -n, a positive dummy-token run, and git diff --check.


View with [code]smith Autofix with [code]smith
Need help on this PR? Tag @codesmith-bot with what you need. Autofix is disabled.

@entelligence-ai-pr-reviews

Copy link
Copy Markdown

EntelligenceAI PR Summary

Updated the release-doctor workflow to pass PRODUCTION_REPO_TOKEN only to the release environment validation step, and aligned the shell check with that current secret name instead of the retired RELEASE_PLEASE_TOKEN. This prevents valid release configurations from failing the safety check while limiting secret exposure.

flowchart TD
    A["Release doctor workflow"] --> B["Check release environment step"]
    C["PRODUCTION_REPO_TOKEN secret"] --> B
    B --> D["check-release-environment script"]
    D --> E["Pass or report missing token"]

    classDef newBehavior fill:#dcfce7,stroke:#16a34a,color:#14532d;
    class A,B,C,D,E newBehavior;
Loading

🟢 Green = new or changed in this PR


Review Scorecard

Dimension Rating Basis
Code Quality ●○○○○ 1/5 — Unacceptable 1 critical finding(s) — reviewer rated the code Unacceptable
Blast Radius Medium touches infra / deploy / config, 2 file(s) / ~6 line(s) changed (size only — not a blast signal)
Merge Confidence ●○○○○ 1/5 — Blocking Issues code quality 1/5 × Medium blast radius

Issues found:

  • Critical .github/workflows/release-doctor.yml — Keep the production token away from PR-controlled code

Not safe to merge — the Medium-blast-radius workflow still exposes a production token to PR-controlled code. Although aligning the check with PRODUCTION_REPO_TOKEN fixes the retired-secret-name mismatch and scopes the variable to one step, that step executes the checked-out bin/check-release-environment, which a pull request can modify to exfiltrate the token. There are no pre-existing unresolved issues involved; this is introduced by the workflow change.

Need to merge before these are addressed? Anyone with write access can comment @entelligence /approve to approve it now. @entelligence help lists every command.

Evaluated against
  • 2/2 changed files reviewed
  • criteria: correctness, security & access control, robustness & error handling, concurrency & data integrity, repo conventions / steering docs
  • steering docs: none found in repo
Files requiring special attention
  • .github/workflows/release-doctor.yml
  • bin/check-release-environment

Comment on lines +18 to +19
env:
PRODUCTION_REPO_TOKEN: ${{ secrets.PRODUCTION_REPO_TOKEN }}

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

CRITICAL SECRET_EXPOSURE Keep the production token away from PR-controlled code

Because this pull_request job executes the checked-out bin/check-release-environment, a same-repository PR named release-please* or next can replace that script and exfiltrate the newly injected PAT. Keep the check on trusted base code before exposing secrets.

Prompt to fix with AI

Copy this prompt into your AI coding assistant to fix this issue.

In .github/workflows/release-doctor.yml at lines 18-19, prevent PRODUCTION_REPO_TOKEN from being available to code supplied by a pull request. Run the release-environment validation from a trusted base/default-branch revision, or redesign the trigger so the secret is only injected after untrusted PR code cannot execute, while preserving validation for legitimate release-please PRs.

@aburkard
aburkard merged commit dc09e88 into main Sep 3, 2026
9 checks passed
@aburkard aburkard mentioned this pull request Sep 3, 2026
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.

1 participant