Skip to content

ci: sanitize PR head ref usage in calm-models publish workflow - #3021

Merged
jpgough-ms merged 1 commit into
finos:mainfrom
rocketstack-matt:fix/osps-br-branch-name-injection
Aug 26, 2026
Merged

ci: sanitize PR head ref usage in calm-models publish workflow#3021
jpgough-ms merged 1 commit into
finos:mainfrom
rocketstack-matt:fix/osps-br-branch-name-injection

Conversation

@rocketstack-matt

@rocketstack-matt rocketstack-matt commented Aug 25, 2026

Copy link
Copy Markdown
Member

Description

Closes the OSPS-BR-01.01 / OSPS-BR-01.02 findings from the LFX Insights build & release review. release-calm-models-maven-publish.yml interpolated github.event.pull_request.head.ref directly into a run: script to extract the release version, and that derived value then flowed the same way into two further shell steps (tag + next-dev-iteration PR). A branch name containing shell metacharacters could inject commands into a job that has contents: write and Maven Central deploy credentials.

Fix: pass the ref and the derived version through env: in all three steps instead of template-interpolating them into the script body, and validate the extracted version against a strict MAJOR.MINOR.PATCH pattern before it's used anywhere — the job already gates on startsWith(head.ref, 'release-prep/calm-models-v'), so this catches any unexpected suffix.

Type of Change

  • 🐛 Bug fix (non-breaking change which fixes an issue)
  • ✨ New feature (non-breaking change which adds functionality)
  • 💥 Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • 📚 Documentation update
  • 🎨 Code style/formatting changes
  • ♻️ Refactoring (no functional changes)
  • ⚡ Performance improvements
  • ✅ Test additions or updates
  • 🔧 Chore (maintenance, dependencies, CI, etc.)

Affected Components

  • CI/CD

Commit Message Format ✅

Used ci: (unscoped fix/feat/perf trigger releases per cli/.releaserc.json's fallback rule; ci is release-neutral).

Testing

  • I have tested my changes locally
  • I have added/updated unit tests
  • All existing tests pass

This workflow only runs on a merged release-prep/calm-models-v* PR against main, so it can't be exercised in CI here. Verified: YAML parses correctly, and the shell logic (prefix-strip, semver validation, env: substitution) was traced by hand against the existing step outputs and Copilot CLI reviewed the diff with no findings.

Checklist

  • My commits follow the conventional commit format
  • I have updated documentation if necessary
  • I have added tests for my changes (if applicable)
  • My changes follow the project's coding standards

…els publish workflow

The Maven Central publish workflow interpolated github.event.pull_request.head.ref
directly into a run: script, and the derived release version then flowed into two
further shell steps the same way. Pass both through env: instead so bash treats
them as data, and validate the extracted version against a strict semver pattern
before it's used to tag, deploy, or open a follow-up PR.

Closes the OSPS-BR-01.01 / OSPS-BR-01.02 findings from the LFX Insights build and
release review.
Copilot AI lite review requested due to automatic review settings August 25, 2026 14:31
@rocketstack-matt
rocketstack-matt requested a review from a team as a code owner August 25, 2026 14:31

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

This PR hardens the release-calm-models-maven-publish.yml GitHub Actions workflow against shell-injection risks by avoiding direct interpolation of untrusted PR branch refs into run: script bodies and by validating the derived release version before use.

Changes:

  • Pass github.event.pull_request.head.ref via env: and derive RELEASE_VERSION inside the shell, rather than embedding the ref directly into the script.
  • Validate the extracted release version against a strict MAJOR.MINOR.PATCH regex before writing it to GITHUB_OUTPUT.
  • Pass the derived RELEASE_VERSION via env: into later steps that tag the release and create the next-development-iteration PR.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

@jpgough-ms
jpgough-ms merged commit b488136 into finos:main Aug 26, 2026
14 checks passed
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.

4 participants