Skip to content

fix: use absolute node path in awf wrapper to survive sudo PATH stripping #25341

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
pelikhan merged 2 commits into from
Apr 8, 2026
+12 −3
Merged

fix: use absolute node path in awf wrapper to survive sudo PATH stripping #25341

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
23c6023
Initial plan
Copilot Apr 8, 2026
951513c
Fix awf wrapper using absolute node path to prevent sudo PATH failure
Copilot Apr 8, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 12 additions & 3 deletions actions/setup/sh/install_awf_binary.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -110,6 +110,12 @@ install_bundle() {
local bundle_name="awf-bundle.js"
local bundle_url="${BASE_URL}/${bundle_name}"

# Capture the absolute path to node so the wrapper works correctly when
# invoked via sudo (where PATH may not include the setup-node install dir,
# e.g. ~/.nvm/versions/node/v24.x.x/bin).
Copy link
Copy Markdown
Contributor

@github-actions github-actions bot Apr 8, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good fix — capturing node_bin with command -v node at wrapper-creation time is the right approach. Consider adding a guard here in case node isn't on PATH yet when this function runs, e.g. if [[ -z "$node_bin" ]]; then echo "ERROR: node not found" >&2; return 1; fi.

local node_bin
node_bin=$(command -v node)
Copy link
Copy Markdown
Contributor

@github-actions github-actions bot Apr 8, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good use of command -v node to capture the absolute path at install time rather than relying on $PATH at runtime. This is the key fix for the sudo PATH-stripping issue — when sudo drops the non-root $PATH, the wrapper script would fail to find node without an absolute path here.


echo "Node.js >= 20 detected ($(node --version)), using lightweight bundle..."
Comment on lines +117 to 119
Copy link

Copilot AI Apr 8, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

node_bin=$(command -v node) does not guarantee an absolute filesystem path: in bash it can return a bare name (or function/alias resolution), which would recreate the original failure mode under sudo secure_path. Prefer node_bin=$(type -P node) (and/or validate [[ -x "$node_bin" ]]) so the wrapper always embeds an executable path or fails fast with a clear error.

Suggested change
node_bin=$(command -v node)
echo "Node.js >= 20 detected ($(node --version)), using lightweight bundle..."
node_bin=$(type -P node || true)
if [[ -z "$node_bin" || ! -x "$node_bin" ]]; then
echo "ERROR: Unable to resolve an executable absolute path for node"
return 1
fi
echo "Node.js >= 20 detected ($("$node_bin" --version)), using lightweight bundle..."

Copilot uses AI. Check for mistakes.
echo "Downloading bundle from ${bundle_url@Q}..."
if ! curl -fsSL --retry 3 --retry-delay 5 -o "${TEMP_DIR}/${bundle_name}" "${bundle_url}"; then
Expand All @@ -127,10 +133,13 @@ install_bundle() {
sudo mkdir -p "${AWF_LIB_DIR}"
sudo cp "${TEMP_DIR}/${bundle_name}" "${AWF_LIB_DIR}/${bundle_name}"

# Create wrapper script
sudo tee "${AWF_INSTALL_DIR}/${AWF_INSTALL_NAME}" > /dev/null <<'WRAPPER'
# Create wrapper script using the absolute path to node.
# Using an unquoted heredoc (<<WRAPPER) so that ${node_bin} is expanded
# at wrapper-creation time, while \$@ is left as the literal $@ for
# runtime argument forwarding.
sudo tee "${AWF_INSTALL_DIR}/${AWF_INSTALL_NAME}" > /dev/null <<WRAPPER
#!/bin/bash
exec node /usr/local/lib/awf/awf-bundle.js "$@"
exec ${node_bin} /usr/local/lib/awf/awf-bundle.js "\$@"
Copy link
Copy Markdown
Contributor

@github-actions github-actions bot Apr 8, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The \$@ escaping is correct for the unquoted heredoc — it ensures $@ is written literally into the wrapper script. Clear comment above explains this well. ✅

Copy link

Copilot AI Apr 8, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The generated wrapper uses exec ${node_bin} ... without quoting. Even if uncommon, an unexpected path containing whitespace or glob characters would break execution due to word-splitting/globbing. Quote the node path in the wrapper (e.g., exec "${node_bin}" ...).

Suggested change
exec ${node_bin} /usr/local/lib/awf/awf-bundle.js "\$@"
exec "${node_bin}" /usr/local/lib/awf/awf-bundle.js "\$@"

Copilot uses AI. Check for mistakes.
Copy link
Copy Markdown
Contributor

@github-actions github-actions bot Apr 8, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The heredoc-expanded \$\{node_bin} correctly hard-codes the absolute node path into the wrapper script at creation time. The comment above (lines 136–138) explaining the unquoted heredoc (<<WRAPPER) vs a quoted one (<<'WRAPPER') is clear and helpful for future maintainers.

WRAPPER
sudo chmod +x "${AWF_INSTALL_DIR}/${AWF_INSTALL_NAME}"

Expand Down
Loading