Skip to content

Pass ref and tag names to the shell through the environment - #164

Merged
kesmit13 merged 2 commits into
mainfrom
ci/quote-ref-names
Nov 13, 2026
Merged

kesmit13 merged 2 commits into
mainfrom
ci/quote-ref-names

Conversation

@kesmit13

@kesmit13 kesmit13 commented Nov 13, 2026 •

Copy link
Copy Markdown
Collaborator

Four workflow steps interpolate tag or branch names straight into run: scripts:

  • publish.yml: "Create GitHub Release" (github.ref_name) and "Upload release assets" (github.event.release.tag_name)
  • fusion-docs.yml: "Upload release asset" (github.event.release.tag_name)
  • code-check.yml: "Check for changes in monitored directories" (github.ref_name, github.event.pull_request.base.ref)

Git ref names may contain $(), backticks and ;, so a crafted tag or branch name would run as shell code. In publish.yml that code runs in the job that holds id-token: write. Creating such a ref needs write access to the repo, so this is hardening rather than an open hole, but the fix is mechanical: pass each name in an environment variable and quote it. code-check.yml already does this for HEAD_REF. Behavior is otherwise unchanged.

The same commit series also moves CLUSTER_PASSWORD and CLUSTER_API_KEY out of three commands and into env. The commands are the create_test_cluster.py calls in publish.yml and smoke-test.yml, and publish.yml's shutdown curl. Those values aren't attacker-controlled, but the shell interpreted them, so a quote, $ or backtick would have broken or altered the command. The drop steps and smoke-test.yml's shutdown already pass them this way. CIBW_ENVIRONMENT is left as is; its comment already requires an alphanumeric password.

CodeRabbit and Cursor Bugbot flagged this on sqlalchemy-singlestoredb#23, which had copied the publish steps from here.

Testing

  • actionlint passes in pre-commit.
  • The same change to the publish steps is in sqlalchemy-singlestoredb#23 and ran end to end there through a test-tag release (v1.2.1-test2), which created the prerelease with its assets.
  • This PR touches publish.yml, so its own run exercises the changed setup-database and shutdown-database steps against a real cluster.
  • No ref-name interpolation is left in a run: script. The only other one, in smoke-test.yml, is inside an if: expression, which is not passed through a shell.

🤖 Generated with Claude Code


Note

Low Risk
Workflow-only quoting/env changes with no application logic impact; reduces shell-injection risk in release and cluster jobs.

Overview
Hardens CI workflows by stopping direct interpolation of branch names, release tags, and API secrets into run: shell scripts. Values now come through quoted environment variables (BASE_REF, REF_NAME, TAG, CLUSTER_PASSWORD, CLUSTER_API_KEY, etc.) so malicious or unusual ref/tag characters ($(), backticks, ;) cannot execute as shell code.

Touched workflows: code-check.yml (PR base vs push comparison), fusion-docs.yml and publish.yml (release create/upload), publish.yml / smoke-test.yml (test cluster provision and teardown). Runtime behavior is unchanged; this is defensive hardening for jobs that already hold sensitive permissions or secrets.

Reviewed by Cursor Bugbot for commit 5e39a5b. Bugbot is set up for automated code reviews on this repo. Configure here.

Summary by CodeRabbit

  • Chores
    • Updated automated checks to compare changes against the appropriate pull request base or push branch.
    • Updated release and documentation publishing workflows to pass release tags consistently through commands.
    • Updated database setup and cluster shutdown workflows to pass credentials through environment variables rather than embedding them directly in commands.
    • These changes affect automated maintenance and publishing processes; no app features are changed.

publish.yml's gh release steps, fusion-docs.yml's asset upload and
code-check.yml's base-commit logic interpolated tag and branch names
straight into their scripts. Ref names may contain $(), backticks and
';', so a crafted tag or branch would run as shell code -- in publish's
case, in the job that holds id-token: write. Pass them in environment
variables and quote them, as code-check already does for HEAD_REF.

Found by review bots on sqlalchemy-singlestoredb #23, which had copied
the publish steps from here.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@coderabbitai

coderabbitai Bot commented Nov 13, 2026 •

Copy link
Copy Markdown

Explain Change →

No actionable comments were generated in the recent review.

Walkthrough

The workflows now pass branch refs, release tags, passwords, and API keys through step environment variables. Shell commands use these variables to select comparison bases, create or shut down clusters, and create or upload releases.

Changes: Workflow environment inputs

Layer / File(s) Summary
Comparison-base selection
.github/workflows/code-check.yml
The step passes the base ref and current ref through environment variables and uses them to select the comparison base.
Cluster credential commands
.github/workflows/publish.yml, .github/workflows/smoke-test.yml
Cluster creation and shutdown commands read passwords and API keys from environment variables instead of embedding secrets in the commands.
Release tag commands
.github/workflows/fusion-docs.yml, .github/workflows/publish.yml
The release workflows pass tags through environment variables and quote them in upload and release creation commands.

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

Change: Refactor

Merge Risk: Minimal · up to 5e39a

The workflow changes pass branch names, tags and credentials through the environment so shell metacharacters in them cannot alter commands. No merge-blocking risk was found.

Security architecture risk: Minimal · up to 5e39a

The changes reduce shell-injection risk without expanding publishing permissions, changing deployment targets, or altering cluster lifecycle controls. No introduced or materially worsened security condition was found.

Retained concerns
No architecture-level concerns identified.

Security review details

Security Blast Radius

  • observed — The affected authority includes repository release writes, the publishing job's existing OIDC and attestation capabilities, and the existing cluster-management credential. The PR adds no permission grants or deployment targets. The actual organization-wide scope of the cluster API key and externally configured publishing trust are not established by repository source.

Security Findings and Attack Paths

  • inferred — The changed release steps remove the path from crafted ref or tag text to shell execution in a privileged publishing job. Quoted environment expansion supplies the text as argument data without recursively interpreting embedded command substitutions. This addresses a pre-existing attack path rather than introducing a new security concern.

Trust Boundaries and Controls

  • observed — The workflow gates excluding fork pull requests from cluster provisioning remain unchanged. Publishing still excludes pull-request and scheduled events and retains its named publish environment. Moving values into environment bindings does not change these repository-defined authorization gates.

Resilience and Maintainability Implications

  • observed — Cleanup remains an attempt rather than a guaranteed terminal state: the jobs use always conditions, reject a missing cluster ID, and fail on a refused deletion. Smoke-test runs retain their non-cancelling concurrency configuration. Interruption, repeated-run behavior, and cleanup recovery are not redesigned by this PR.
Pre-merge checks: Passed 4

Check name Status Explanation
Title check Passed The title "Pass ref and tag names to the shell through the environment" matches the main change. The PR moves branch and tag names out of run: script interpolation in the GitHub Actions workflows and …
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.
Description Check Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Full details: Title check

Explanation

The title "Pass ref and tag names to the shell through the environment" matches the main change. The PR moves branch and tag names out of run: script interpolation in the GitHub Actions workflows and passes them through environment variables. The title does not mention the secret handling in create_test_cluster.py and the shutdown curl command, but the title does not need to cover every detail.

Finishing Touches

Generate unit tests (beta)

  • Commit to this branch
  • Create a new PR
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
Recent review info

Run configuration

  • Source mode: Auto
  • Repository: Unavailable
  • Branch: Unavailable
  • File: No configuration file from this repository applied
  • Commit: Unavailable
  • Layers: defaults
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: 322fb739-bec6-43a6-933a-53216d4b5ab9
Commits

Reviewing files that changed from the base of the PR and between 56cd248 and 5e39a5b.

Files selected for processing: 2

  • .github/workflows/publish.yml
  • .github/workflows/smoke-test.yml

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

The create_test_cluster.py calls in publish.yml and smoke-test.yml, and
publish.yml's shutdown curl, interpolated CLUSTER_PASSWORD and
CLUSTER_API_KEY into their commands, so a quote, $ or backtick in either
would break or alter the command. Pass them in environment variables,
as the drop steps and smoke-test.yml's shutdown already do.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🟢 Approval recommended

No active review issues are reported for the workflow shell-safety changes.

No open issues 🎉

What changed in this PR

Hardens GitHub Actions workflows by passing ref names, release tags, and cluster credentials through environment variables instead of interpolating them directly into shell scripts. This prevents their contents from being interpreted as shell code while preserving intended workflow behavior.

Changes:

  • Uses quoted environment variables for branch comparisons and release creation/uploads.
  • Moves cluster credentials into step-level environment variables for provisioning and shutdown.
  • Adds comments explaining the shell-safety rationale.
File Description
.github/​workflows/​smoke-test.yml Passes cluster provisioning credentials through environment variables.
.github/​workflows/​publish.yml Safely passes release tags and cluster credentials to shell commands.
.github/​workflows/​fusion-docs.yml Uses a quoted tag environment variable for documentation asset uploads.
.github/​workflows/​code-check.yml Uses environment variables for event and branch comparisons.

Review effort: Balanced

@kesmit13
kesmit13 merged commit 2e06a02 into main Nov 13, 2026
22 checks passed
@kesmit13
kesmit13 deleted the ci/quote-ref-names branch November 13, 2026 16:42

This branch was previously deployed

1 inactive deployment
Base — 5e39a5b9 Deployed Nov 13, 2026 by kesmit13 via test-coverage #654
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