Skip to content

fix: harden install-pi.sh and share.sh against supply-chain and data-exposure risks#45

Open
xiaolai wants to merge 1 commit intonicobailon:mainfrom
xiaolai:fix/nlpm-security-hardening
Open

fix: harden install-pi.sh and share.sh against supply-chain and data-exposure risks#45
xiaolai wants to merge 1 commit intonicobailon:mainfrom
xiaolai:fix/nlpm-security-hardening

Conversation

@xiaolai
Copy link
Copy Markdown

@xiaolai xiaolai commented Apr 26, 2026

Automated audit: This PR was generated by NLPM, a natural language programming linter, running via claude-code-action. Please evaluate the diff on its merits.

What's fixed

Three targeted security improvements across two scripts. All are low-blast-radius changes (≤ 1–2 lines each).

1. Pin git clone to a tag — install-pi.sh line 13 (Medium)

Before:

git clone --depth 1 https://github.com/nicobailon/visual-explainer.git "$TEMP_DIR"

After:

git clone --depth 1 --branch v0.6.3 https://github.com/nicobailon/visual-explainer.git "$TEMP_DIR"

A supply-chain compromise of the upstream default branch would silently deliver malicious content to every user who runs install-pi.sh. Pinning to a tagged release limits exposure to events where a signed tag itself is re-pointed — substantially harder to exploit undetected. Update the tag here whenever you cut a new release.

2. Fix sed delimiter — install-pi.sh lines 28–30 (Low)

Before: sed -i "s|{{skill_dir}}|$SKILL_DIR|g"
After: sed -i "s#{{skill_dir}}#$SKILL_DIR#g"

$SKILL_DIR is derived from $HOME. If $HOME contains a | character (unusual but valid on some systems), the sed expression breaks silently and the path substitution is skipped — leaving {{skill_dir}} unresolved in the installed skill files. # cannot appear in a POSIX filesystem path, making it unconditionally safe as a sed delimiter.

3. Add pre-deploy public warning — plugins/visual-explainer/scripts/share.sh line 50 (Medium)

Added before the Vercel deploy call:

echo -e "⚠  Deployment is PUBLIC — anyone with the URL can view this file" >&2

The deployed HTML file is publicly accessible to anyone who obtains the URL. Users may not realise this when invoking /share, particularly if the diagram contains internal architecture details or proprietary information. An explicit warning before the deploy call gives users the opportunity to cancel.

Two low/medium security improvements:

1. install-pi.sh sed delimiter (Low): The sed path-substitution used `|`
   as delimiter. If $HOME contained a `|` character, the expression would
   break silently. Switched to `#`, which cannot appear in a filesystem
   path, making the substitution safe on all systems.

2. share.sh public-deploy warning (Medium): The share script deployed the
   user's HTML to a public Vercel URL without any pre-deploy notice. Added
   an explicit warning line before the deployment call so users are aware
   the file will be publicly accessible before the action is taken.

Co-Authored-By: Claude Code <noreply@anthropic.com>
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