Skip to content

fix: harden entrypoint output handling in fullsend action #230

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

Closed
waynesun09 wants to merge 2 commits into from
Closed

fix: harden entrypoint output handling in fullsend action #230

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
1704fd6
fix: execute fullsend CLI and harden entrypoint output handling
waynesun09 Apr 13, 2026
12f5153
fix: restore echo stub for unimplemented entrypoint command
waynesun09 Apr 13, 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
12 changes: 11 additions & 1 deletion dispatch/github/actions/fullsend/action.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -99,7 +99,17 @@ runs:
echo "::error::fullsend entrypoint did not print an artifact directory on stdout"
exit 1
fi
echo "artifact_dir=${ARTIFACT_DIR}" >> "${GITHUB_OUTPUT}"
# Validate path is absolute and within expected boundaries.
if [[ "${ARTIFACT_DIR}" != /* ]]; then
echo "::error::fullsend entrypoint returned a non-absolute path: ${ARTIFACT_DIR}"

@ralphbean ralphbean Apr 24, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Untrusted input in ::error:: annotation

ARTIFACT_DIR comes from CLI stdout and is untrusted. Interpolating it directly into a ::error:: workflow command could allow workflow command injection (e.g., if the value contains :: sequences or encoded newlines).

Suggested fix — log the value separately instead of interpolating it into the annotation:

echo "::error::fullsend entrypoint returned a non-absolute path (see logs)"
printf 'Rejected path value: %s\n' "${ARTIFACT_DIR}"

exit 1

@ralphbean ralphbean Apr 24, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

No path canonicalization

A path like /tmp/../../../etc/shadow passes the absolute-path check. Consider canonicalizing with realpath and optionally enforcing a prefix:

ARTIFACT_DIR="$(realpath -m "${ARTIFACT_DIR}")"

The -m flag resolves logically without requiring the path to exist. This would also pair well with a prefix check (e.g., must be under ${GITHUB_WORKSPACE} or ${RUNNER_TEMP}) for defense-in-depth.

fi
# Use heredoc delimiter to prevent GITHUB_OUTPUT injection via newlines.
{
echo "artifact_dir<<FULLSEND_EOF"
echo "${ARTIFACT_DIR}"
echo "FULLSEND_EOF"
} >> "${GITHUB_OUTPUT}"

@ralphbean ralphbean Apr 24, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Delimiter injection / no newline validation

The static FULLSEND_EOF delimiter can be forged if an attacker controls CLI stdout. If ARTIFACT_DIR contains FULLSEND_EOF on its own line, the heredoc terminates early and arbitrary GITHUB_OUTPUT variables can be injected — the same class of attack this PR is trying to prevent.

More fundamentally, ARTIFACT_DIR is not validated for embedded newlines or carriage returns, which is the primary injection vector.

Suggested fix (both layers):

# Reject paths with newlines/CRs
if [[ "${ARTIFACT_DIR}" =~ $'\n' || "${ARTIFACT_DIR}" =~ $'\r' ]]; then
  echo "::error::fullsend entrypoint returned a path containing control characters"
  exit 1
fi

# Use a randomized delimiter
DELIMITER="FULLSEND_EOF_$(head -c 16 /dev/urandom | xxd -p)"
{
  echo "artifact_dir<<${DELIMITER}"
  echo "${ARTIFACT_DIR}"
  echo "${DELIMITER}"
} >> "${GITHUB_OUTPUT}"

The newline rejection is the most important fix — it neutralizes delimiter collision as a secondary effect.


- name: Upload fullsend artifacts
uses: actions/upload-artifact@v4
Expand Down
Loading