ci: harden workflows against shell injection

[hyunhee-jo]

Summary

• sync-upstream.yml: Escape $VERSION in sed to prevent special character injection, replace heredoc with echo block to avoid unquoted variable expansion, quote gh pr create arguments with ${}
• release.yml: Use shell variable ${VERSION} instead of ${{ env.VERSION }} in run: block to prevent injection

Context

Security audit of CI workflows found that $VERSION (derived from git tags or workflow inputs) was used unescaped in sed patterns and unquoted in shell commands. While git tags are typically controlled, this pattern is vulnerable to special characters (/, &, \) breaking or exploiting sed commands.

${{ env.VERSION }} in run: blocks is a known shell injection vector per GitHub Security Lab guidance. Using shell variables (${VERSION}) from env: blocks is the safe pattern.

Test plan

• actionlint .github/workflows/*.yml passes
• Trigger sync-upstream.yml via workflow_dispatch with a test version
• Verify release.yml builds correctly on manual trigger

Summary by CodeRabbit

• Chores
  • Improved CI workflow reliability for build/release steps: safer version propagation, more robust handling of special characters in version strings, and more reliable PR creation and messaging.

---

Note: This release contains internal infrastructure updates with no user-facing feature changes.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro

Run ID: 68f297bd-c95c-4040-9f96-5f58b0199780

📥 Commits

Reviewing files that changed from the base of the PR and between ddaf25c and aebeb41.

📒 Files selected for processing (2)

• .github/workflows/release.yml
• .github/workflows/sync-upstream.yml

---

Walkthrough

Updated GitHub Actions workflows to improve shell interpolation and safe substitution of the release version: added an escaped intermediate ESCAPED_VER for sed replacements, tightened variable quoting/bracing across steps, and changed PR body generation to use an echo-based write in sync-upstream.yml.

Changes

Cohort / File(s)	Summary
Release workflow .github/workflows/release.yml	Replaced direct interpolation with an intermediate ESCAPED_VER that escapes sed metacharacters (&, /, \) before using it in the sed replacement targeting pyproject.toml's version line.
Sync-upstream workflow — variable interpolation & quoting .github/workflows/sync-upstream.yml	Replaced $VERSION with ${VERSION} in multiple steps and added quoting where appropriate (validation, generator --version, branch/commit messages, PyPI wait/install, PR title).
Sync-upstream workflow — safe sed substitution .github/workflows/sync-upstream.yml	Added ESCAPED_VER computed from VERSION by escaping /, &, and \ and used it in the sed replacement when updating pyproject.toml.
Sync-upstream workflow — PR body and creation .github/workflows/sync-upstream.yml	Converted PR body construction from a heredoc to grouped echo writes into /tmp/pr-body.md; changed gh pr create invocation to pass ${DRAFT_FLAG} unquoted (so empty yields no arg) and to quote --label "${LABELS}"; PR title now uses ${VERSION}.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

🚥 Pre-merge checks | ✅ 3

✅ Passed checks (3 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title 'ci: harden workflows against shell injection' directly and accurately summarizes the main objective of the changeset—to address shell injection vulnerabilities in CI workflows by escaping variables and improving quoting patterns.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Inline comments:
In @.github/workflows/release.yml:
- Line 43: The sed invocation that replaces version in pyproject.toml currently
interpolates ${VERSION} unescaped, which breaks if VERSION contains sed
metacharacters; before the sed -i "s/^version = \".*\"/version =
\"${VERSION}\"/" call, create an escaped variable (e.g., ESCAPED_VER) by piping
VERSION through printf and a sed that escapes &, / and \ (same approach used in
sync-upstream.yml), then use ${ESCAPED_VER} in the substitution so the sed
replacement is safe; keep the target file (pyproject.toml) and the surrounding
sed command names (VERSION, ESCAPED_VER) to locate and replace the line.

🪄 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: Organization UI

Review profile: ASSERTIVE

Plan: Pro

Run ID: 569cc284-a6e8-41a8-a377-43873ea11ded

📥 Commits

Reviewing files that changed from the base of the PR and between f252560 and ddaf25c.

📒 Files selected for processing (2)

• .github/workflows/release.yml
• .github/workflows/sync-upstream.yml

[claude]

Changelog

All notable changes to langchain-opendataloader-pdf will be documented in this file.

Format follows Keep a Changelog.

[Unreleased]

Added

• detect_strikethrough parameter (from opendataloader-pdf v2.1.0)
• Sync markers for automated option synchronization with opendataloader-pdf
• CI: test.yml — unit tests on every PR (Python 3.10, 3.12, 3.13)
• CI: test-full.yml — multi-platform full test (manual trigger)
• CI: sync-upstream.yml — automated code generation + PR on upstream release
• Regression snapshot tests
• pytest-socket for network isolation in unit tests

Changed

• split_pages parameter moved after synced params block (keyword-only usage unaffected)
• hybrid_timeout default documented as "0" (no timeout), was "30000"
• README AI-AGENT-SUMMARY license: MIT → Apache-2.0
• README Parameters Reference table now auto-generated from options.json

Fixed

• Parameters Reference table missing detect_strikethrough

[2.0.0] - 2026-03-26

Added

• Hybrid AI extraction mode (hybrid, hybrid_mode, hybrid_url, hybrid_timeout, hybrid_fallback)
• sanitize parameter for sensitive data masking
• pages parameter for page selection
• include_header_footer parameter
• split_pages parameter with per-page Document splitting
• password parameter for encrypted PDFs
• use_struct_tree parameter for tagged PDFs
• table_method parameter (default, cluster)
• reading_order parameter (xycut, off)
• image_output, image_format, image_dir parameters
• keep_line_breaks parameter
• replace_invalid_chars parameter
• JSON page splitting by page number field
• Comprehensive unit tests (59 tests)
• Integration tests with real PDF files (24 tests)
• Sample PDFs for testing

Changed

• License changed from MIT to Apache-2.0
• Build system changed from poetry to hatchling
• Dependency: opendataloader-pdf>=2.0.0 (was >=1.3.0)

[1.2.0] - 2025-12-15

Changed

• Updated for opendataloader-pdf v1.3.0 compatibility

[1.1.0] - 2025-11-20

Changed

• Migrated from run() to convert() API
• File-based output processing instead of in-memory

[1.0.0] - 2025-10-01

Added

• Initial release
• OpenDataLoaderPDFLoader class with BaseLoader interface
• Text and JSON output format support
• lazy_load() iterator support

[claude]

Changelog

All notable changes to langchain-opendataloader-pdf will be documented in this file.

Format follows Keep a Changelog.

[Unreleased]

Added

• detect_strikethrough parameter (from opendataloader-pdf v2.1.0)
• Sync markers for automated option synchronization with opendataloader-pdf
• CI: test.yml — unit tests on every PR (Python 3.10, 3.12, 3.13)
• CI: test-full.yml — multi-platform full test (manual trigger)
• CI: sync-upstream.yml — automated code generation + PR on upstream release
• Regression snapshot tests
• pytest-socket for network isolation in unit tests

Changed

• split_pages parameter moved after synced params block (keyword-only usage unaffected)
• hybrid_timeout default documented as "0" (no timeout), was "30000"
• README AI-AGENT-SUMMARY license: MIT → Apache-2.0
• README Parameters Reference table now auto-generated from options.json

Fixed

• Parameters Reference table missing detect_strikethrough

[2.0.0] - 2026-03-26

Added

• Hybrid AI extraction mode (hybrid, hybrid_mode, hybrid_url, hybrid_timeout, hybrid_fallback)
• sanitize parameter for sensitive data masking
• pages parameter for page selection
• include_header_footer parameter
• split_pages parameter with per-page Document splitting
• password parameter for encrypted PDFs
• use_struct_tree parameter for tagged PDFs
• table_method parameter (default, cluster)
• reading_order parameter (xycut, off)
• image_output, image_format, image_dir parameters
• keep_line_breaks parameter
• replace_invalid_chars parameter
• JSON page splitting by page number field
• Comprehensive unit tests (59 tests)
• Integration tests with real PDF files (24 tests)
• Sample PDFs for testing

Changed

• License changed from MIT to Apache-2.0
• Build system changed from poetry to hatchling
• Dependency: opendataloader-pdf>=2.0.0 (was >=1.3.0)

[1.2.0] - 2025-12-15

Changed

• Updated for opendataloader-pdf v1.3.0 compatibility

[1.1.0] - 2025-11-20

Changed

• Migrated from run() to convert() API
• File-based output processing instead of in-memory

[1.0.0] - 2025-10-01

Added

• Initial release
• OpenDataLoaderPDFLoader class with BaseLoader interface
• Text and JSON output format support
• lazy_load() iterator support

[claude]

Code Review

This is a well-motivated security hardening PR. The changes follow GitHub Security Lab guidance and improve the CI workflows.

---

What is good

release.yml -- ${{ env.VERSION }} to ${VERSION}

This is the most impactful fix. Interpolating ${{ env.VERSION }} directly into a run: block is a known shell injection vector -- the value is substituted into the shell script as raw text before execution. Using ${VERSION} from the shell environment instead is the correct pattern. Since VERSION is written to $GITHUB_ENV in a prior step, it is correctly available to all subsequent steps.

sync-upstream.yml -- ESCAPED_VER for sed replacement

Good defense-in-depth. Escaping &, /, and \ prevents those characters from being interpreted as sed metacharacters in the replacement string.

Echo block instead of heredoc

Cleaner and removes ambiguity around heredoc delimiter quoting. The previous << EDOFOBODY (unquoted) already expanded variables, so there was no active injection risk -- but the echo block is more explicit and easier to reason about.

"${LABELS}" quoting

Correct -- quotes prevent word splitting on the comma-separated label string.

---

Issues and suggestions

1. ESCAPED_VER is redundant given the semver validation (minor)

The version is already validated against ^[0-9]+\.[0-9]+\.[0-9]+$ at the top of the generate job, which rules out all sed-special characters. The escaping is not wrong (defense-in-depth is fine) but a comment explaining the intent alongside the existing validation would help future readers.

2. ${DRAFT_FLAG} is unquoted -- intentional but undocumented

The unquoted form is actually correct here: when DRAFT_FLAG is empty, the unquoted expansion produces no argument (desired), while "${DRAFT_FLAG}" would pass an empty string to gh pr create (would error). However, shellcheck will flag this and it reads like a mistake. A comment helps, or use an array pattern:

FLAGS=(--base main --label "${LABELS}")
[ "$STATUS_DRAFT" == "true" ] && FLAGS+=(--draft)
gh pr create --title "..." --body-file /tmp/pr-body.md "${FLAGS[@]}"

3. Remaining unbraced $VERSION in shell contexts

A few occurrences were not updated:

• Line 117: BRANCH="sync/v$VERSION"
• Line 124: git commit -m "sync: update to opendataloader-pdf v$VERSION"

Not a security issue given the semver validation, but inconsistent with the PR goal of using ${} braces throughout.

4. echo "${STATUS_RESULT}" -- consider printf

printf '%s\n' "${STATUS_RESULT}" is more portable than echo. Not a real risk here since the value is always one of two hardcoded strings, but worth the habit for values that could contain backslash sequences.

---

Test plan note

The PR mentions running actionlint -- it would be reassuring to see that output or have it added as a CI check. Linting workflow files on every PR would catch regressions automatically.

---

Summary

The core fixes are correct and this is a net improvement to CI security posture. Main suggestions:

1. Add a comment to ESCAPED_VER acknowledging it is defense-in-depth alongside the semver validation
2. Document or restructure the intentionally-unquoted ${DRAFT_FLAG}
3. Apply brace consistency to the remaining $VERSION in shell contexts

Happy to approve once those points are addressed or acknowledged.

[hyunhee-jo]

Addressed in aebeb41:

1. ESCAPED_VER defense-in-depth comment ✅ Added to both sync-upstream.yml and release.yml.

2. DRAFT_FLAG unquote comment ✅ Explains why it must remain unquoted.

3. Brace consistency ✅ All bare $VERSION → ${VERSION} across both files.

4. release.yml sed escape ✅ Applied same ESCAPED_VER pattern as sync-upstream.yml.

Skipped:

• echo → printf: Fixed strings only, no backslash risk.

Independent sub-agent review confirmed all 5 checks PASS.

[claude]

Follow-up Review

All four action items from the previous review have been addressed in aebeb41:

• ESCAPED_VER comment ✅ — Both release.yml and sync-upstream.yml now have the defense-in-depth comment clarifying why escaping is applied even though semver validation already guards against metacharacters.
• DRAFT_FLAG comment ✅ — The intentionally-unquoted expansion is now explained inline, which will also serve as a note for anyone running shellcheck.
• Brace consistency ✅ — All remaining bare $VERSION occurrences (branch name, commit message, pip install loop) are now ${VERSION}.
• release.yml ESCAPED_VER ✅ — Same pattern applied, consistent with sync-upstream.yml.

The echo → printf skip for ${STATUS_RESULT} is reasonable given the value is a controlled two-option string.

The PR is in good shape. No further blocking issues.