security: reject symlinks in PR-supplied Firebase preview artifact by wecbaiyk-blip · Pull Request #3654 · angular/dev-infra

wecbaiyk-blip · 2026-05-09T00:29:36Z

Summary

upload-artifacts-to-firebase extracts a PR-supplied tarball (unsafe-artifact.zip -> deploy-artifact.tar.gz) into the Firebase public directory and then deploys it to a Firebase Hosting preview channel. The artifact is explicitly annotated as RISK in the action's own comments, and the description points at the GitHub Security Lab guidance on preventing pwn requests.

This PR adds a defense-in-depth check after extraction: if firebase-public-dir ends up containing any symlinks, the deploy step fails fast with an explanatory error.

Why

Currently tar -xvzf materialises any symlinks the PR author put in their artifact onto the runner's firebase-public-dir. Downstream tooling that walks firebase-public-dir to enumerate the files to upload (e.g. firebase-tools listFiles) calls fs.readFile* on each entry, which follows symlinks at the OS layer. A tar entry like

public/leak -> /proc/self/environ
public/leak -> ~/.config/gcloud/application_default_credentials.json
public/leak -> ~/.ssh/id_rsa
public/leak -> /run/secrets/<anything>

would therefore have the target's bytes uploaded to the publicly-readable preview CDN under a path the attacker chose.

There is no legitimate reason for a static-hosting public/ artifact to contain symlinks (the Hosting CDN serves files, not symlinks), so refusing them outright is safer than enumerating "safe" symlink targets.

Changes

In the Extracting workflow artifact into Firebase public directory. step:

Add set -euo pipefail so a shell error during extraction does not silently fall through to the deploy step.
After tar -xvzf, run find "$publicDir" -type l and exit non-zero with a ::error:: annotation if any entry is found. -type l catches symlink-to-file and symlink-to-directory regardless of whether their target is reachable.
No-op when the artifact is well-formed (no symlinks).

The check intentionally fails loudly rather than silently deleting symlinks, so the contributor sees the security policy and the maintainer can audit any legitimate edge case.

Backward compatibility

Workflows whose preview artifacts contain only regular files and directories are unaffected. Workflows that intentionally include symlinks in their preview artifact would start failing - but the maintainers' own framing (RISK annotation, pwn-request reference) suggests symlinks were never an intended use case.

Why not rely on the upstream `firebase-tools` fix alone

There are in-flight firebase-tools PRs on the same threat model:

firebase/firebase-tools#10477 - default-flip on fsAsync.readdirRecursive ignoreSymlinks.
firebase/firebase-tools#10478 - drop symlinks from Hosting deploy upload list (listFiles).

Those fix the same bug class at the deployment-tool layer. This PR is the action-layer belt-and-suspenders: even if those land later, get partially reverted, or are bypassed by a downstream tool that re-introduces symlink-following, the artifact never reaches the deploy stage with symlinks present in the first place.

Testing

Manual validation locally:

mkdir -p /tmp/pub
echo "<!doctype html>" > /tmp/pub/index.html
ln -s /etc/passwd /tmp/pub/leak
# new check rejects:
find /tmp/pub -type l   # prints /tmp/pub/leak

mkdir -p /tmp/pub2
echo "<!doctype html>" > /tmp/pub2/index.html
# new check passes:
find /tmp/pub2 -type l  # prints nothing

CI for this action runs in the Angular dev-infra integration suite; no test changes are needed because the action is shell-based and the new step is exercised end-to-end whenever a preview deploy runs.

`upload-artifacts-to-firebase` extracts a PR-supplied tarball (`unsafe-artifact.zip` -> `deploy-artifact.tar.gz`) into the Firebase public directory and then deploys it to a Firebase Hosting preview channel. The artifact is explicitly annotated as `RISK` in this action's comments and the README points at GitHub Security Lab's guidance on preventing pwn requests. Currently `tar -xvzf` materialises any symlinks that the PR author chose to put in their artifact onto the runner's `firebase-public-dir`. Downstream tooling that walks `firebase-public-dir` to enumerate the files to upload (e.g. `firebase-tools` `listFiles`) calls `fs.readFile*` on each entry, which follows symlinks at the OS layer. A tar entry like `public/leak -> /proc/self/environ` or `public/leak -> ~/.config/gcloud/application_default_credentials.json` would therefore have its target's bytes uploaded to the publicly-readable preview CDN. Defense in depth: after extraction, fail the deploy if the public directory contains any symlinks. There is no legitimate reason for a static-hosting artifact to contain symlinks, so refusing them outright is safer than trying to enumerate safe symlink targets. The check uses `find -type l` which detects both symlink-to-file and symlink-to-directory entries regardless of whether their target is reachable. `set -euo pipefail` is added at the top of the step so a shell error during extraction does not silently fall through to the deploy step. This patch complements upstream `firebase-tools` hardening (separate PRs against `firebase/firebase-tools`) and stays defensive even if those land later or are partially reverted.

gemini-code-assist

Code Review

This pull request introduces security hardening to the Firebase artifact upload process by adding 'set -euo pipefail' and a symlink detection check to prevent data leakage. The review feedback identifies several opportunities to further improve security, including preventing shell injection by passing inputs via environment variables, ensuring the symlink check does not fail-open, and sanitizing output to prevent malicious filenames from being interpreted as workflow commands.

gemini-code-assist · 2026-05-09T00:31:40Z

+        # `public` artifact destined for static hosting to contain
+        # symlinks, so refusing them outright is the safest default.
+        # See https://securitylab.github.com/research/github-actions-preventing-pwn-requests/.
+        symlinks=$(find "$publicDir" -type l 2>/dev/null || true)


The || true and 2>/dev/null suffixes make this security check "fail-open". If the find command fails for any reason (e.g., due to permission errors on directories created by the untrusted artifact), the symlinks variable will be empty, and the check will silently pass. \n\nSince set -e is active, removing || true ensures the workflow fails if the check cannot be completed. Removing 2>/dev/null ensures that any errors encountered during the scan are visible in the logs for debugging.

symlinks=$(find "$publicDir" -type l)

gemini-code-assist · 2026-05-09T00:31:40Z

+        set -euo pipefail
+
        extractDir="$RUNNER_TEMP/artifact-unpack"
+        publicDir='${{inputs.firebase-public-dir}}'


Interpolating ${{ inputs.firebase-public-dir }} directly into the shell script is vulnerable to shell injection if the input contains single quotes (e.g., path'with'quotes). Although this input is marked as TRUSTED in this repository, it is a security best practice in GitHub Actions to pass inputs via environment variables to avoid them being interpreted as part of the script body.

gemini-code-assist · 2026-05-09T00:31:40Z


        unzip unsafe-artifact.zip -d "$extractDir"
-        tar -xvzf "$extractDir/deploy-artifact.tar.gz" -C '${{inputs.firebase-public-dir}}'
+        tar -xvzf "$extractDir/deploy-artifact.tar.gz" -C "$publicDir"


Consider removing the -v (verbose) flag from the tar command. Since the artifact is PR-supplied and potentially malicious, a filename containing a newline followed by a GitHub Actions workflow command (e.g., \n::error ...) could be interpreted by the runner when printed to stdout during extraction.

tar -xzf "$extractDir/deploy-artifact.tar.gz" -C "$publicDir"

gemini-code-assist · 2026-05-09T00:31:40Z

+        if [ -n "$symlinks" ]; then
+          echo "::error title=Symlinks rejected in artifact::The deploy artifact contains symlinks, which are not permitted in a Hosting public directory for security reasons (a symlink can leak the contents of files outside the source tree onto the public Firebase Hosting CDN)."
+          echo "Symlinks found:"
+          echo "$symlinks"


When printing the list of found symlinks, it is safer to indent the output. This prevents a malicious filename containing a newline from being interpreted as a GitHub Actions workflow command if it happens to start at the beginning of a line. Using printf is also safer than echo for printing arbitrary strings.

printf '%s\n' "$symlinks" | sed 's/^/ /'

gemini-code-assist Bot reviewed May 9, 2026

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

security: reject symlinks in PR-supplied Firebase preview artifact#3654

security: reject symlinks in PR-supplied Firebase preview artifact#3654
wecbaiyk-blip wants to merge 1 commit intoangular:mainfrom
wecbaiyk-blip:security/upload-artifacts-reject-symlinks

wecbaiyk-blip commented May 9, 2026

Uh oh!

gemini-code-assist Bot left a comment

Uh oh!

gemini-code-assist Bot May 9, 2026

Uh oh!

gemini-code-assist Bot May 9, 2026

Uh oh!

gemini-code-assist Bot May 9, 2026

Uh oh!

gemini-code-assist Bot May 9, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Conversation

wecbaiyk-blip commented May 9, 2026

Summary

Why

Changes

Backward compatibility

Why not rely on the upstream firebase-tools fix alone

Testing

Uh oh!

gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

Code Review

Uh oh!

gemini-code-assist Bot May 9, 2026

Choose a reason for hiding this comment

Uh oh!

gemini-code-assist Bot May 9, 2026

Choose a reason for hiding this comment

Uh oh!

gemini-code-assist Bot May 9, 2026

Choose a reason for hiding this comment

Uh oh!

gemini-code-assist Bot May 9, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Why not rely on the upstream `firebase-tools` fix alone