Skip to content

fix: harden deploy.sh and export-pdf.sh#96

Open
lukesw55 wants to merge 2 commits into
zarazhangrui:mainfrom
lukesw55:fix/script-hardening
Open

fix: harden deploy.sh and export-pdf.sh#96
lukesw55 wants to merge 2 commits into
zarazhangrui:mainfrom
lukesw55:fix/script-hardening

Conversation

@lukesw55

@lukesw55 lukesw55 commented Jul 2, 2026

Copy link
Copy Markdown

Fixes the issues described in #95.

export-pdf.sh

  • static server binds 127.0.0.1 only
  • request paths are resolved against the serve root; anything escaping it gets a 403
  • browser and server are closed in try/finally, so failed exports no longer leave Chromium running
  • animations disabled during screenshots for deterministic output

deploy.sh

  • asset references parsed by a small Python helper instead of grep/sed; refs resolving outside the HTML's directory are skipped, missing ones are reported
  • temp dirs cleaned via EXIT trap on every exit path
  • Vercel CLI invoked as an array (no word-splitting), npx-only; the npm install -g fallback is gone
  • deploy-URL extraction prefers vercel.app / now.sh URLs before falling back to the last URL in the output

No interface changes: same arguments, same flow, same user-facing messages. scripts/ and the plugins/frontend-slides/skills/frontend-slides/scripts/ copies are updated in sync.

bash -n passes on all four scripts. The containment logic lives in the Python (deploy) and Node (export) helpers and is small enough to review directly in the diff.

- parse HTML asset refs in Python with path-traversal protection
  (normpath + containment check; rejects ../, absolute and // refs)
- clean temp dirs via EXIT trap on all exit paths
- invoke Vercel CLI as array to avoid word-splitting
- drop the npm install -g global fallback
- extract the deploy URL more robustly
- static server listens on 127.0.0.1 instead of all interfaces
- reject path-traversal requests with 403
- close browser and server in try/finally
- disable animations for deterministic screenshots
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