Skip to content

Comments

scripts/make-site.sh: avoid eval and validate DOCSY_VERS to prevent command injection#2422

Open
thesmartshadow wants to merge 9 commits intogoogle:mainfrom
thesmartshadow:fix/remove-eval-docsy-vers
Open

scripts/make-site.sh: avoid eval and validate DOCSY_VERS to prevent command injection#2422
thesmartshadow wants to merge 9 commits intogoogle:mainfrom
thesmartshadow:fix/remove-eval-docsy-vers

Conversation

@thesmartshadow
Copy link

Summary

This PR hardens scripts/make-site.sh by removing unsafe shell eval usage and validating user-controlled inputs (notably -v / DOCSY_VERS) to prevent OS command injection during Hugo module operations.

Background / Root Cause

DOCSY_VERS is provided via the -v flag and is incorporated into the Hugo module reference. The script previously used eval for Hugo module commands, which can turn attacker-influenced data into shell syntax (command separators / metacharacters), resulting in unintended command execution.

Fix

  • Remove eval from Hugo / Hugo-module invocation paths and pass arguments safely.
  • Add conservative allowlist validation for DOCSY_VERS and DOCSY_REPO (reject whitespace / shell metacharacters and unexpected patterns).
  • Preserve existing behavior for valid refs/tags/branches; invalid inputs now fail fast with a clear error.

Why this matters (realistic threat model)

Docsy’s own documentation references automated build/publish workflows (e.g., PR deploy previews). In real build/preview pipelines, values such as refs/branches/tags and other derived parameters can become attacker-influenced (directly or indirectly) and reach scripts like make-site.sh. When DOCSY_VERS is concatenated into a string executed via eval, shell metacharacters can be interpreted as commands (build-time OS command injection). This can impact the CI runner, secrets available to the job, and the integrity of generated/published artifacts (supply-chain risk).

Security impact

This is a build-time hardening change. If -v is influenced by untrusted CI inputs or build automation parameters, the previous eval usage could enable command execution in the build environment, potentially impacting:

  • CI runner integrity
  • secrets exposure
  • artifact integrity (supply-chain risk)

Security classification

  • CWE-78: OS Command Injection
  • Contributing weakness: unsafe use of eval with attacker-influenced data

Testing

  • Verified: ./scripts/make-site.sh -s HUGO -v main still works as expected.
  • Verified: malicious/invalid -v values are rejected and do not get executed.

References

  • CWE-78: OS Command Injection

@thesmartshadow
Copy link
Author

Hi maintainers this PR removes unsafe eval usage in scripts/make-site.sh and validates -v (DOCSY_VERS) to prevent shell metacharacter injection in Hugo module operations.

Current status shows:

  • “Review required” (needs 1 approving review from a maintainer)
  • “Workflows awaiting approval” (CI for forks requires maintainer approval)

If you can:

  1. Approve/run the pending workflows, and
  2. Review the change,

I verified:

  • ./scripts/make-site.sh -s HUGO -v main still works
  • malicious/invalid -v values are rejected and not executed

Thanks!

@thesmartshadow
Copy link
Author

Hi maintainers
Just a gentle follow-up from an external contributor.
This PR removes unsafe eval usage in scripts/make-site.sh and adds validation for -v / DOCSY_VERS to avoid shell metacharacter injection in Hugo module operations.

All checks are green except workflows that require maintainer approval.
Happy to adjust anything if needed. Thanks!

@chalin
Copy link
Collaborator

chalin commented Jan 19, 2026

HI @thesmartshadow. Thanks for the PR. Given that this is a script for internal usage, I'll be focusing on other priorities before I can get to this one. I'll get back to it as soon as I can.

@thesmartshadow
Copy link
Author

Hi @chalin, thanks a lot for the update and for taking a look.

Totally understand that there are other priorities and that this script is mainly used internally.
The goal of this change is just to harden the build-time path in case scripts/make-site.sh is ever wired into CI or other automated workflows where -v / DOCSY_VERS might come from derived inputs (refs, metadata, etc.). For normal usage the behavior should stay the same, and invalid values now fail fast with a clear error instead of reaching eval.

No rush on my side happy to wait, and also happy to adjust or split the patch if there’s anything you’d like changed.

Thanks again!

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