Skip to content

ci: feedz.io empty-key wording + pass push keys via env (#8600 follow-up) - #1121

Merged
sfmskywalker merged 1 commit into
mainfrom
fix/8600-feedz-wording-env-key
Oct 4, 2026
Merged

sfmskywalker merged 1 commit into
mainfrom
fix/8600-feedz-wording-env-key

Conversation

@sfmskywalker

@sfmskywalker sfmskywalker commented Oct 4, 2026 •

Copy link
Copy Markdown
Member

Follow-up to elsa-workflows/elsa-core#8600 (Code Review non-blocking items on the empty-key guard PR).

  • N1: the feedz.io empty-key error now names the FEEDZ_API_KEY repository secret only. That job never uses the NuGet login (OIDC) step. The nuget.org error now points at the OIDC login output and the Trusted Publishing policy.
  • N2: both dotnet nuget push steps take the key from a step-level env: API_KEY and use "$API_KEY", instead of interpolating ${{ }} into the command line.

No behaviour change otherwise. Each check still reads exactly what its push uses, and the conditions are unchanged.

Summary by CodeRabbit

  • Reliability
    • Publishing to Feedz and nuget.org now checks that an API key is available before attempting to publish and provides a more specific error when configuration is missing.

@coderabbitai

coderabbitai Bot commented Oct 4, 2026 •

Copy link
Copy Markdown

Review in Change Stack →

Navigate logical layers of code changes, visualize relationships, and explore their blast radius.

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration
  • Configuration used: defaults
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: 4a6e361e-c19b-482b-b3b5-19fef2b5b194
📥 Commits

Reviewing files that changed from the base of the PR and between 7355b1b and 90f5b8b.

📒 Files selected for processing (1)
  • .github/workflows/packages.yml

Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 6 remain after this review.


📝 Walkthrough

Walkthrough

The Feedz and nuget.org publishing steps now pass API keys through step-scoped API_KEY environment variables. Empty-key error messages identify the relevant secret or publishing configuration.

Changes

Package publishing

Layer / File(s) Summary
Publisher key validation and use
.github/workflows/packages.yml
The Feedz and nuget.org steps check for empty keys and update their error messages. Each step passes its key through API_KEY and quotes that variable in the push command.

Priority: ⬇️ Low

Estimated code review effort: 2 (Simple) | ~8 minutes

Change: Other

Suggested reviewers: cursoragent

Merge Risk: ⚪ Minimal · up to 90f5b

No actionable merge-blocking risk is established in the changed publishing workflow.

Architecture Summary

Architecture risk: 🔵 Low · up to 90f5b

The changed surface does not map to a changed system, dependency edge, entrypoint, or external dependency.

Changed systems: None identified.

Architecture concerns
No architecture-level concerns identified.

Review details

Before / after behavior

  • observed — Modified behavior in .github/workflows/packages.yml: The Feedz check now directs empty-key errors to the FEEDZ_API_KEY secret, and the publish step exposes that secret as API_KEY and quotes the variable in the push command instead of interpolating the secret directly.
  • observed — Modified behavior in .github/workflows/packages.yml: The nuget.org check now directs empty-key errors to the NuGet login output and Trusted Publishing policy. The publish step exposes the OIDC-derived key as API_KEY and quotes it in the push command instead of interpolating the output directly.
🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly identifies the Feedz.io error wording and environment-based NuGet push keys as the main changes.
Description check ✅ Passed The description explains the problem and solution for both changes. It does not include verification steps or select a scope checkbox, but the main purpose and technical details are clear.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff. Analyzed 0 functions across 0…
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Commit to this branch
  • Create a new PR
  • Autopilot · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Comment @coderabbitai help to get the list of available commands.

@greptile-apps

greptile-apps Bot commented Oct 4, 2026 •

Copy link
Copy Markdown
Contributor

RetriggerConfidence Score: 5/5

[High risk] CI workflow passes secrets via environment variables instead of command-line arguments.

Safe to merge based on the workflow changes checked.

What we checked:

  • Executed the parent and updated workflow check-step scripts with missing, empty, and dummy-present keys using a stubbed dotnet command, and confirmed the updated workflow produced service-specific errors and stopped before publishing with dummy-present keys reaching the stub once per job. T-Rex
  • Executed both publish commands from the parent and updated workflows with dummy keys containing spaces, shell metacharacters, and newlines using a stubbed dotnet command and dummy package, and confirmed the key arrived as a single argument in all eight updated-workflow cases while shell text did not execute and the parent did not preserve the key arguments. T-Rex
  • From the repository checkout, ran empty-key-guard-harness.py HEAD^ and HEAD; both exited 0, but at HEAD missing and empty keys produced the intended service-specific diagnostics and skipped the publish step, with zero calls to the stubbed dotnet and dummy-present keys reaching the stub once per job. T-Rex
  • From the repository checkout, ran nuget-key-argument-test.py HEAD^ and HEAD; both harness runs exited 0, HEAD preserved the single key argument and related fields in all eight cases, while the parent failed the argument-preservation property and did not inject commands. T-Rex

Summary

The feedz.io and nuget.org publish steps now give service-specific errors for empty keys and pass keys through environment variables rather than interpolating them into shell commands. Isolated checks confirmed that empty keys stop publishing and that the changed commands preserve each key as one argument.

Reviews (1) · Last reviewed commit: "ci: name FEEDZ_API_KEY in feedz.io empty..."

@sfmskywalker sfmskywalker left a comment

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Elsa 3 Code Review: APPROVE + HIGH @ 90f5b8b

Code Review, Round 1/4

Scope: .github/workflows/packages.yml +8/-4. Fixes N1 and N2 from #1120 (elsa-core#8600 follow-up).

Verdict: No blockers.

Checks

  • Same source as before.
    • Publish to feedz.io now has env: API_KEY: ${{ secrets.FEEDZ_API_KEY }}, the expression it previously put on the command line. It matches its guard.
    • Publish to nuget.org now has env: API_KEY: ${{ steps.nuget_login.outputs.NUGET_API_KEY }}, the same OIDC output as before and as its guard.
    • Nothing reads secrets.NUGET_API_KEY.
  • Quoting and exposure.
    • The key is passed as -k "$API_KEY" / --api-key "$API_KEY", double-quoted.
    • No ${{ }} key expression is left on any command line.
    • Nothing echoes the key, and there is no set -x.
  • Conditions. The if: lines are identical to the base, including the !startsWith(needs.build.outputs.version, '3.10.') guard on publish_nuget_nuget. No step-level if: was added.
  • Messages.
    • feedz.io now names only FEEDZ_API_KEY.
    • nuget.org now points at the OIDC login output and the Trusted Publishing policy.
    • Both still say nothing was pushed and are followed by exit 1.
  • actionlint. 1.7.7 with shellcheck reports 16 findings on both base and head, and none are new.
  • Consistency with elsa-extensions#277. The four steps match line for line: same env:, same guard text, same quoting. The only differences are pre-existing ones: the package download path, the --api-key/--source vs -k/-s flags, and the env var name for the feed URL.

Non-blocking

  • N1. "repository secret" is not accurate here. The feedz.io message says "Check the FEEDZ_API_KEY repository secret". For elsa-studio, FEEDZ_API_KEY exists only at organization level; there is no repository-level copy. Saying "the FEEDZ_API_KEY secret (repository or organization)" would point operators to the right place. Keep the wording the same as in extensions.

Bots and CI on 90f5b8bb

  • CI: all green: Build and test, CodeQL (all Analyze jobs), GitGuardian and CLA. Merge state is CLEAN.
  • Greptile: 5/5 at 90f5b8bb (advisory).
  • CodeRabbit: no actionable comments.
  • Bugbot: did not run.
  • Threads: none.

Gate: APPROVE + HIGH and green CI on 90f5b8bb. Met.

@sfmskywalker
sfmskywalker merged commit 1b31030 into main Oct 4, 2026
11 checks passed
@sfmskywalker
sfmskywalker deleted the fix/8600-feedz-wording-env-key branch October 4, 2026 03:31
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