Skip to content

security(cookbook): harden patch execution by removing shell interpolation#3662

Open
mateeaaaaaaa wants to merge 1 commit intoShopify:mainfrom
mateeaaaaaaa:main
Open

security(cookbook): harden patch execution by removing shell interpolation#3662
mateeaaaaaaa wants to merge 1 commit intoShopify:mainfrom
mateeaaaaaaa:main

Conversation

@mateeaaaaaaa
Copy link
Copy Markdown

WHY are these changes introduced?

Recipe patch application was using a shell-interpolated command string when
running patch. Using shell strings for dynamic file paths is a command
injection risk pattern.

This PR hardens that path by switching to argv-based process execution.

WHAT is this pull request doing?

  • Replaces shell-interpolated patch execution with argv-based execution in
    cookbook/src/lib/apply.ts.
  • Changes import from execSync to execFileSync for this call site.
  • Keeps recipe behavior unchanged while removing shell parsing from dynamic
    path inputs.

Security impact:

  • Reduces command injection risk in recipe patch application.
  • Follows secure spawn guidance: avoid shell command construction, pass dynamic
    values as argv.

HOW to test your changes?

  1. Install dependencies from the repository root:
    • corepack pnpm install --filter cookbook...
  2. Run the cookbook apply tests:
    • cd cookbook
    • corepack pnpm vitest run src/lib/apply.test.ts
  3. Run Semgrep on the changed file:
    • semgrep scan --config p/security-audit cookbook/src/lib/apply.ts

Expected results:

  • Vitest: src/lib/apply.test.ts passes (2 tests).
  • Semgrep: no findings reported for cookbook/src/lib/apply.ts.

Post-merge steps

None.

Checklist

  • I've read the Contributing Guidelines
  • I've considered possible cross-platform impacts (Mac, Linux, Windows)
  • I've added a changeset if this PR contains user-facing or functional changes. Test changes or internal-only config changes do not require a changeset.
  • I've added tests to cover my changes
  • I've added or updated the documentation

@mateeaaaaaaa mateeaaaaaaa requested a review from a team as a code owner April 1, 2026 21:44
@shopify
Copy link
Copy Markdown
Contributor

shopify bot commented Apr 1, 2026

Oxygen deployed a preview of your main branch. Details:

Storefront Status Preview link Deployment details Last update (UTC)
Skeleton (skeleton.hydrogen.shop) ✅ Successful (Logs) Preview deployment Inspect deployment April 2, 2026 4:01 AM
metaobjects ✅ Successful (Logs) Preview deployment Inspect deployment April 2, 2026 4:02 AM
classic-remix ✅ Successful (Logs) Preview deployment Inspect deployment April 1, 2026 9:44 PM
third-party-queries-caching ✅ Successful (Logs) Preview deployment Inspect deployment April 2, 2026 4:02 AM
custom-cart-method ✅ Successful (Logs) Preview deployment Inspect deployment April 2, 2026 4:02 AM
sitemap ✅ Successful (Logs) Preview deployment Inspect deployment April 2, 2026 4:02 AM

Learn more about Hydrogen's GitHub integration.

@mateeaaaaaaa
Copy link
Copy Markdown
Author

I have signed the CLA!

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.

1 participant