chore: pin @sistent/sistent exact in meshery-extensions dependency bump#1727
Conversation
meshery-extensions pins externalized deps (shared runtime singletons with Meshery UI) to exact versions, unlike meshery, meshery-cloud, and layer5, which use caret ranges for the same packages. This bot's plain npm install wrote a caret, breaking that convention and triggering the same Copilot/Gemini review comment on every sistent release (e.g. meshery-extensions#4300). Add --save-exact to the bump-meshery-extensions job only; the other jobs' conventions are unaffected. meshery-extensions also gets a belt-and-suspenders ui/.npmrc with save-exact=true and a CI check (validate-mesheryui-compatibility.js) enforcing this regardless of which caller runs npm install. Signed-off-by: Ahmed Jamil <197998703+CodeAhmedJamil@users.noreply.github.com>
|
Note Gemini is unable to generate a review for this pull request due to the file types involved not being currently supported. |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughThe dependent notification workflow strictly validates ChangesDependent release version workflow
Estimated code review effort: 2 (Simple) | ~10 minutes 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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.
Inline comments:
In @.github/workflows/notify-dependents.yml:
- Line 128: Update the npm install command in the workflow step to validate
RELEASE_VERSION against a strict semver pattern before use and safely quote the
validated value in the shell command. Reject invalid workflow_dispatch input
before invoking npm, while preserving the existing exact-version installation
behavior.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: d07dea9d-cbd9-4c6f-959e-5eca025b0198
📒 Files selected for processing (1)
.github/workflows/notify-dependents.yml
The bump jobs interpolated ${{ env.RELEASE_VERSION }} directly into the
`npm install` command line, so the Actions template engine pasted the value
into the shell script before bash ever saw it. The value reaches that point
unvalidated: the workflow_dispatch input is free text, and a git refname may
legally contain $, backticks, parentheses and quotes.
Fix at the source rather than at the four call sites:
- The info job now validates the resolved version against an anchored semver
pattern and fails closed. It is the single choke point every bump job reads
from, so one check covers all of them.
- The branch-name fallback regex was unanchored, so it also matched (and then
used wholesale) strings that merely contained something version-shaped -
e.g. `release/v0.21.40` was accepted as a version and handed to npm.
Anchoring it fixes that latent bug and closes the injection path together.
- All four `run:` steps now reference the job-level env var as a quoted shell
variable instead of a template expansion.
zizmor reports 8 template-injection findings on this file before the change
and 0 after.
Signed-off-by: Ahmed Jamil <197998703+CodeAhmedJamil@users.noreply.github.com>
|
Addressed the one actionable review item (CodeRabbit's template-injection finding on Acknowledging the two informational bot comments for completeness: Gemini reported it cannot review these file types (no action available), and the CodeRabbit walkthrough is a summary with all five pre-merge checks passing (no action required). Out-of-scope note for maintainers: @copilot review |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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.
Inline comments:
In @.github/workflows/notify-dependents.yml:
- Line 39: Strengthen the SEMVER_RE validation in the workflow so it enforces
the full SemVer grammar: reject leading zeroes in numeric components, numeric
prerelease identifiers with leading zeroes, and empty prerelease or build
identifiers such as consecutive dots. Use a strict parser or equivalent anchored
regex while preserving acceptance of valid SemVer values before the npm install
steps.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 7ba5bd7f-ba03-4637-86e1-b582fa9bc6a1
📒 Files selected for processing (1)
.github/workflows/notify-dependents.yml
The first pass used a permissive pattern that accepted 01.2.3, 1.2.3-01 and 1.2.3..rc. None of those carry shell metacharacters, so the injection fix held, but a validation gate that accepts values its own name says are invalid is a trap for the next reader - and these would have reached npm and failed there instead of here, with a worse error. Use the canonical SemVer 2.0.0 grammar from semver.org, transcribed to POSIX ERE and assembled from named parts so it stays readable: no leading zeroes in numeric identifiers, no empty prerelease or build identifiers. Verified by extracting the info job's script from the workflow and running it: 11 valid versions accepted; 15 malformed values and 7 shell-injection payloads rejected, across all three resolution paths (dispatch input, branch name, API fallback). zizmor still reports 0 template-injection findings on this file. Signed-off-by: Ahmed Jamil <197998703+CodeAhmedJamil@users.noreply.github.com>
Problem
Every
@sistent/sistentbump PR this workflow opens againstlayer5labs/meshery-extensionsgets the identical Copilot/Gemini review comment asking to pin the dependency to an exact version — see meshery-extensions#4300, and every priorbump-sistent-botPR before it.meshery-extensionspins externalized dependencies (packages shared as runtime singletons with Meshery UI, including@sistent/sistent) to exact versions inui/package.json— unlikemeshery/meshery,meshery-cloud, andlayer5, which all use caret ranges for the same packages. The plainnpm install @sistent/sistent@Xin thebump-meshery-extensionsjob writes a caret by default, silently breaking that repo's convention on every release.Fix
Add
--save-exactto thebump-meshery-extensionsjob'snpm install, matching the job's existing--legacy-peer-depsper-repo customization. The other three jobs (bump-meshery,bump-layer5,bump-meshery-cloud) are untouched — their target repos use caret ranges by convention, so this would be an unwanted change there.This is a second line of defense. The primary fix lives in
meshery-extensionsitself (ui/.npmrcwithsave-exact=true, plus a new CI check that fails the build if any externalized dependency is ever range-pinned again, regardless of source) — see layer5labs/meshery-extensions#4301.Test plan
python3 -c "import yaml; yaml.safe_load(...)"— workflow YAML still parses--save-exact) to one job; no other jobs touchedSummary by CodeRabbit