Skip to content

Route release.yml version through env indirection; pin dev/docs deps - #2

Merged
carochacs merged 3 commits into
mainfrom
claude/security-issues-jdpj26
Aug 7, 2026
Merged

Route release.yml version through env indirection; pin dev/docs deps#2
carochacs merged 3 commits into
mainfrom
claude/security-issues-jdpj26

Conversation

@carochacs

@carochacs carochacs commented Aug 6, 2026

Copy link
Copy Markdown
Collaborator

What

Two LOW hardening items found during a security audit:

1. Unpinned shell interpolation in release.yml. ${{ steps.ver.outputs.version }} was interpolated directly into two run: shell steps — the classic GitHub Actions shell-injection pattern. Not exploitable today (the value is constrained by changelog_extract.py's \d+\.\d+\.\d+ regex), but fragile if that regex or its input ever changes. Routed through an env: VERSION var instead, matching the pattern already used in plugin-validate.yml.

2. Unpinned dev/docs dependencies. requirements-dev.txt / requirements-docs.txt left jsonschema, pytest, ruff, and mkdocs-material fully unpinned, so CI silently picks up whatever's newest on a given day — a breaking change upstream shows up as a CI failure unrelated to whatever PR triggered it. Pinned to minor-version-compatible ranges (~=X.Y).

Verified: fresh pip install -r requirements-dev.txt, full pytest suite (11 passed), ruff check tools/ tests/ (clean), tools/validate.py examples/minimal-plugin examples/full-plugin (both ok), and tools/check_versions.py (consistent) all pass with the pinned versions. release.yml YAML parses cleanly.

Checklist

  • No behavior change to the release workflow's logic, only how the version string reaches the shell
  • Existing test/lint/validate suite passes

Generated by Claude Code

Greptile Summary

This PR routes the extracted release version through step environment variables and constrains development and documentation dependencies.

  • Replaces direct GitHub-expression interpolation in release shell commands with quoted $VERSION expansion.
  • Adds compatible-release constraints for jsonschema, pytest, ruff, and mkdocs-material.
  • The dependency specifiers are broader than the comments’ intended minor-series pins.

Confidence Score: 4/5

The PR appears safe to merge, though its dependency ranges do not fully implement the intended minor-series pinning.

The workflow hardening preserves behavior, while the dependency constraints leave non-blocking future CI drift possible because two-component compatible-release specifiers admit later minor releases.

Files Needing Attention: requirements-dev.txt and requirements-docs.txt

Important Files Changed

Filename Overview
.github/workflows/release.yml Safely routes the release version through an environment variable without changing release behavior.
requirements-dev.txt Adds dependency constraints, but their two-component compatible-release syntax permits later minor versions contrary to the stated policy.
requirements-docs.txt Constrains mkdocs-material, but the range similarly permits every later 9.x minor release.

Fix All in Claude Code Fix All in Codex

Prompt To Fix All With AI
### Issue 1
requirements-dev.txt:10-12
**Ranges permit later minor releases**

Two-component compatible-release constraints such as `~=4.26` and `~=9.1` permit all later releases within the same major version, so CI can still silently adopt newer minor versions despite the comments requiring deliberate minor-version upgrades; the same issue applies to `mkdocs-material~=9.7` in `requirements-docs.txt`.

---

For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.

Reviews (1): Last reviewed commit: "Route release.yml version through env in..." | Re-trigger Greptile

Greptile also left 1 inline comment on this PR.

Two LOW hardening items found during a security audit:

- release.yml interpolated ${{ steps.ver.outputs.version }} directly
  into two run: shell steps — the classic GitHub Actions
  shell-injection pattern. Not exploitable today (the value is
  constrained by changelog_extract.py's \d+\.\d+\.\d+ regex), but
  fragile if that regex or its input ever changes. Routed through an
  env: VERSION var instead, matching the pattern already used in
  plugin-validate.yml.

- requirements-dev.txt / requirements-docs.txt left jsonschema,
  pytest, ruff, and mkdocs-material fully unpinned, so CI silently
  picks up whatever's newest on a given day. Pinned to
  minor-version-compatible ranges (~=X.Y). Verified: fresh install,
  full pytest suite, ruff, tools/validate.py against both examples,
  and tools/check_versions.py all still pass.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_018J1NvPtPQZd3cEt6aAbGzG
@coderabbitai

coderabbitai Bot commented Aug 6, 2026

Copy link
Copy Markdown

Important

Review skipped

Auto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 8f35f7d7-6091-444f-b0d8-f5fffc30a2e6

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review

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

Comment thread requirements-dev.txt Outdated
Greptile review on this PR: PEP 440's compatible-release operator
needs all three version components to lock a minor version.
~=4.26 (two components) is NOT equivalent to "4.26.x only" — it means
>=4.26, ==4.* and happily permits 4.27, 4.99, anything in the 4.x
line, same as leaving it unpinned in every way that actually matters
here. Verified directly against packaging.specifiers.SpecifierSet:
~=4.26 accepts 4.27.0, ~=4.26.0 correctly rejects it.

Rewrote all four pins with the third component
(jsonschema~=4.26.0, pytest~=9.1.1, ruff~=0.16.1,
mkdocs-material~=9.7.7), matching the exact versions already verified
installable in the prior commit, and added a comment explaining why
the third digit is load-bearing so this doesn't regress again.

Verified: fresh pip install, full pytest suite (11 passed), ruff
clean, tools/validate.py against both examples, tools/check_versions.py
all still pass with the corrected pins. Confirmed via SpecifierSet
that each pin now allows patch-level drift but rejects the next minor
version.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_018J1NvPtPQZd3cEt6aAbGzG

@greptile-apps greptile-apps Bot 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.

carochacs has reached the 50-credit limit for trial accounts. To continue receiving code reviews, upgrade your plan.

@pullfrog pullfrog Bot 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.

Review summary

The Greptile Issue 1 is valid and correctly resolved by commit 6797719.

Dependency pins~=X.Y~=X.Y.Z is the right fix. Under PEP 440's compatible-release clause, ~=4.26.0 means >=4.26.0, ==4.26.*: it locks the minor version and allows patch-only drift, while ~=4.26 admits any 4.*. Verified with packaging.specifiers.SpecifierSet:

  • jsonschema~=4.26.0 rejects 4.27.0
  • pytest~=9.1.1 rejects 9.2.0
  • ruff~=0.16.1 rejects 0.17.0
  • mkdocs-material~=9.7.7 rejects 9.8.0

All four pinned versions exist on PyPI. The added comment explaining why the third component is load-bearing is a good guard against regression.

release.yml — routing the version output through env: VERSION and using quoted "${VERSION}" in the shell steps is correct and matches the plugin-validate.yml pattern. The if: conditions keep the ${{ steps.ver.outputs.version }} GitHub-expression form, which the runner evaluates (not the shell), so there's no injection surface there.

Verification (with the pinned versions installed)pytest: 11 passed; ruff check tools/ tests/: clean; tools/validate.py examples/minimal-plugin examples/full-plugin: both ok; tools/check_versions.py: consistent.

Non-blocking observation (outside this PR's diff)

The lint job in .github/workflows/validate.yml still does python -m pip install --upgrade ruff — an unpinned install, so that job remains exposed to exactly the silent CI drift this PR eliminates for the validate and docs jobs (a future ruff release can newly flag existing code and fail the check). If full pinning is the goal, that step could install -r requirements-dev.txt instead. Not blocking — the PR's own changes are correct and mergeable as-is.

Pullfrog  | Fix it ➔View workflow run | Using Big Pickle (free) | 𝕏

@pullfrog

pullfrog Bot commented Aug 7, 2026

Copy link
Copy Markdown

Review 4879002473 contained no open threads — the review body confirmed Greptile's Issue 1 (dependency ranges) was already correctly resolved by commit 6797719, and flagged one non-blocking observation. I addressed it: the lint job in .github/workflows/validate.yml was still installing ruff unpinned (pip install --upgrade ruff); it now installs -r requirements-dev.txt, matching the validate/docs jobs and eliminating the remaining silent-CI-drift surface this PR targets. Pushed as commit 83f34a4.

Verified: full suite passes (11 tests), ruff check tools/ tests/ clean, both example plugins validate, versions consistent, and all workflow YAML files parse.

Task list (1/4 completed)
  • Checkout PR branch and inspect review comments
  • Verify and address the dependency pinning finding
  • Test/lint, review diff, commit
  • Push, reply + resolve threads, report progress

Pullfrog  | View workflow run | via Pullfrog | Using Big Pickle (free) | 𝕏

@carochacs
carochacs merged commit 3d653d1 into main Aug 7, 2026
8 checks passed
@carochacs
carochacs deleted the claude/security-issues-jdpj26 branch August 7, 2026 01:56
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