Keep provider key scoped to ZERO step#448
Conversation
There was a problem hiding this comment.
Pull request overview
This pull request tightens secret handling in the ZERO GitHub Action by ensuring provider API keys are scoped to the single “Run ZERO” step (rather than being persisted for later steps), and updates accompanying documentation and smoke validation to match.
Changes:
- Stop persisting the provider API key via
GITHUB_ENV, while still exporting it in-process for the ZERO execution step. - Update action/docs wording to explicitly state the key is exported only for the ZERO step (and not inherited by later steps).
- Extend the smoke workflow to validate the non-persistence behavior and to run when the action documentation changes.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| docs/GITHUB_ACTION.md | Updates docs to clarify that provider API keys are exported only for the ZERO step and not persisted to later steps. |
| action.yml | Removes persistence of provider credentials to later steps by eliminating writes to GITHUB_ENV, keeping only same-step export for ZERO. |
| .github/workflows/zero-action-smoke.yml | Adds checks to ensure the action exports the provider key only within the Run ZERO step and does not persist it via GITHUB_ENV; triggers also include doc changes. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
WalkthroughThis PR changes the composite GitHub Action to export the provider API key only within the "Run ZERO" step instead of persisting it to GITHUB_ENV for later steps. Documentation is updated to reflect this scoping, and the CI smoke test workflow adds path triggers and assertions validating the new behavior. ChangesZERO step API key scoping
Estimated code review effort: 2 (Simple) | ~15 minutes Sequence Diagram(s)sequenceDiagram
participant CIWorkflow as Zero Action Smoke CI
participant ActionYml as action.yml
participant RunZeroStep as Run ZERO step
participant LaterSteps as Later workflow steps
CIWorkflow->>ActionYml: Trigger on push/PR to changed paths
ActionYml->>RunZeroStep: Export provider API key (step-scoped)
RunZeroStep-->>LaterSteps: Key not persisted to GITHUB_ENV
CIWorkflow->>ActionYml: Validate raw file content
CIWorkflow->>CIWorkflow: Assert no inheritance text, no GITHUB_ENV write
Related Issues: None specified Related PRs: None specified Suggested labels: security, documentation, ci Suggested reviewers: None specified Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
There was a problem hiding this comment.
🧹 Nitpick comments (1)
.github/workflows/zero-action-smoke.yml (1)
87-101: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueMinor duplication: re-parses
action.ymlinstead of reusingaction_raw.This block re-opens and re-parses
action.ymlindependently of theaction_raw/docalready produced in the prior validation step. Not a functional problem since these run as separate Python invocations, but consider centralizing the parse if these checks grow.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.github/workflows/zero-action-smoke.yml around lines 87 - 101, The shell-syntax-check step in the workflow re-parses action.yml instead of reusing the earlier parsed data from the validation logic. Update the Python block in the Shell-syntax-check embedded run blocks step to centralize or share the existing action.yml parse flow used by the prior validation step, keeping the behavior the same but avoiding duplicated parsing as the checks evolve.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In @.github/workflows/zero-action-smoke.yml:
- Around line 87-101: The shell-syntax-check step in the workflow re-parses
action.yml instead of reusing the earlier parsed data from the validation logic.
Update the Python block in the Shell-syntax-check embedded run blocks step to
centralize or share the existing action.yml parse flow used by the prior
validation step, keeping the behavior the same but avoiding duplicated parsing
as the checks evolve.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 262712f7-e10e-4dbc-a54b-bf4e9b1225fd
📒 Files selected for processing (3)
.github/workflows/zero-action-smoke.ymlaction.ymldocs/GITHUB_ACTION.md
Vasanthdev2004
left a comment
There was a problem hiding this comment.
Approve — clean, correct security hardening
Worth it: yes. This closes a genuine credential-exposure gap in the GitHub Action: the provider API key was written to GITHUB_ENV, which persists it into the environment of every subsequent step in the job — including any third-party actions a user runs after ZERO. Removing that write scopes the key to the single Run ZERO step, where it's still available in-process via export. Thanks @GautamBytes.
No regression: zero exec runs inside the same Run ZERO step, so the in-process export is all the provider needs. I confirmed no later step consumes the key — the PR-comment step uses GH_TOKEN, the Slack step uses SLACK_WEBHOOK_URL, neither touches the provider credential.
Security check:
- Key now lives only in the Run ZERO process env —
grepconfirms zero remainingGITHUB_ENVwrites of anyAPI_KEYvar. ✓ - Fail-closed unchanged: the
api-key-envname is still validated against^[A-Za-z_][A-Za-z0-9_]*$before export. ✓ - No plaintext downgrade:
set +x, noechoof the value, never logged. ✓ - Regression guard added: the smoke workflow now asserts the key is still exported for the step, is never persisted to
GITHUB_ENV, and that docs no longer claim later steps inherit it.
I ran the new smoke assertions locally against the PR's action.yml (pyyaml) — all pass. No Go surface. CI fully green including the Windows smoke (3m53s), which is exactly what guards this change.
Non-blocking: the "later steps inherit" not in action_raw.lower() assertion is a string-literal guard tied to the old comment wording; the line-level GITHUB_ENV+API_KEY check below it is the real protection. And worth a one-line release note that any downstream workflow relying on the previously-leaked env var will now see the key unset (that leak was the vulnerability). Neither blocks. Tightly-scoped hardening — approving.
gnanam1990
left a comment
There was a problem hiding this comment.
VERDICT: approve
REGRESSION RISK: low
The change removes a single line from action.yml that wrote the provider API key to GITHUB_ENV (printf '%s=%s\n' ... >> "${GITHUB_ENV}"). The in-process export for the same step is retained, so zero exec still sees the key. The effect is that the key no longer persists into subsequent job steps — a security hardening, not a behavior regression for the ZERO step itself. The smoke workflow adds assertions verifying the key is not written to GITHUB_ENV and that the Run ZERO step exports it in-process. Docs are updated to match.
BUILD / TEST
go build ./...— pass (no Go files changed; only YAML and Markdown)go vet ./...— pass- CI all green, including the
Validate action.ymljob which now enforces the new assertions - The smoke test's Python validation now checks: (1) the Run ZERO step exports the key, (2) the key is not persisted to
GITHUB_ENV, (3) docs don't claim later steps inherit it
CONTRIBUTING
No linked issue. The author is a community contributor (GautamBytes). Per CONTRIBUTING.md, community PRs require an approved parent issue with the issue-approved label. No such issue is linked. This is a process violation, though the change is a legitimate security hardening. Maintainer's call.
FINDINGS
- Nit —
.github/workflows/zero-action-smoke.yml:87-101— minor duplication in Python validation. The script re-reads and re-parsesaction.ymlintoaction_rawafter already loadingdocfromyaml.safe_load. This works but could reuse the already-parseddocfor the structural checks. Not a blocker — the duplication is harmless and the assertions are correct.
No other findings.
EXISTING REVIEWS
- Vasanthdev2004 (APPROVED): Assessed as clean, correct security hardening closing a genuine credential-exposure gap. Valid on the current head. Nothing missed.
- coderabbitai (APPROVED + 1 nitpick): Flagged the same Python duplication. Valid, trivial. Nothing missed.
- copilot-pull-request-reviewer (COMMENTED): Accurate overview of the change. Nothing missed.
All reviews on the current head (b8a1abc). No stale reviews.
BOTTOM LINE
A clean security fix that stops persisting provider API keys to GITHUB_ENV; safe to merge, noting the missing issue link for maintainer awareness.
Summary:
Summary by CodeRabbit
Bug Fixes
Documentation
api-key,api-key-env, and related settings behave.