Skip to content

ci(preview): add a preview-public label to drop the oauth2 gate #138

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

Open
jon-bell wants to merge 1 commit into
base: main
Choose a base branch
from
Open

ci(preview): add a preview-public label to drop the oauth2 gate #138

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
37 changes: 34 additions & 3 deletions .github/workflows/preview-deploy.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,9 +13,13 @@ name: Preview Deploy
# - Fork PR: a maintainer adds the `preview-deploy` label. From that
# point, pushes to the PR redeploy automatically while the label is
# present. Removing the label tears down (see preview-teardown.yml).
#
# Previews are additionally gated behind oauth2-proxy (GitHub, ndif-team
# org-only) by default. A maintainer can add the `preview-public` label
# to drop that gate for a single PR — see the `auth_gate` output below.
on:
pull_request_target:
types: [opened, reopened, synchronize, labeled]
types: [opened, reopened, synchronize, labeled, unlabeled]
Comment on lines 20 to +22

@coderabbitai coderabbitai Bot Aug 10, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🔒 Security & Privacy | 🟠 Major | 🏗️ Heavy lift

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

file=".github/workflows/preview-deploy.yml"

rg -n -C 6 \
  'pull_request_target|head\.sha|actions/checkout|docker/build-push-action|helm upgrade|PREVIEW_KUBECONFIG|NDIF_API_KEY|HF_TOKEN' \
  "$file"

Repository: ndif-team/workbench

Length of output: 8284

🏁 Script executed:

#!/bin/bash
set -euo pipefail

file=".github/workflows/preview-deploy.yml"

printf '%s\n' '--- workflow structure and permissions ---'
sed -n '1,180p' "$file"
printf '%s\n' '--- build and deployment steps ---'
sed -n '180,345p' "$file"

printf '%s\n' '--- referenced build and chart files ---'
for f in workbench/_web/Dockerfile workbench/_api/Dockerfile deploy/preview/Chart.yaml deploy/preview/values.yaml; do
  if [ -f "$f" ]; then
    echo "### $f"
    sed -n '1,220p' "$f"
  else
    echo "### $f (not present)"
  fi
done

printf '%s\n' '--- relevant workflow references ---'
rg -n -C 4 'permissions:|checkout|docker/build-push-action|docker/login|secrets\.|helm upgrade|deploy/preview|kubectl|run:' "$file"

Repository: ndif-team/workbench

Length of output: 32514

🏁 Script executed:

#!/bin/bash
set -euo pipefail

printf '%s\n' '--- preview chart files ---'
find deploy/preview -maxdepth 3 -type f -print | sort
printf '%s\n' '--- secret and workload references in the chart ---'
rg -n -C 5 'secret|env:|envFrom|volume|volumeMount|Deployment|Job|Pod|initContainer|image:' deploy/preview

printf '%s\n' '--- API startup and secret consumers ---'
rg -n -C 5 'NDIF_API_KEY|HF_TOKEN|os\.environ|environ|getenv|set_default_api_key|uvicorn|FastAPI|main' workbench/_api

printf '%s\n' '--- workflow checkout and secret-bearing command boundaries ---'
awk '
  /actions\/checkout@/ || /Provision api secrets/ || /Helm upgrade --install/ ||
  /secrets\./ || /helm upgrade/ || /docker\/build-push-action/ {
    start = NR - 3
    if (start < 1) start = 1
    end = NR + 8
    printf "\n--- lines %d-%d ---\n", start, end
    for (i = start; i <= end; i++) lines[i] = 1
  }
  { text[NR] = $0 }
  END {
    for (i = 1; i <= NR; i++)
      if (lines[i]) printf "%4d %s\n", i, text[i]
  }
' .github/workflows/preview-deploy.yml

Repository: ndif-team/workbench

Length of output: 31084

Do not run PR-controlled code in this secret-bearing pull_request_target job.

The job checks out github.event.pull_request.head.sha, builds PR-controlled Dockerfiles, and deploys the PR-controlled deploy/preview chart. The chart injects NDIF_API_KEY and HF_TOKEN into preview workloads. A labeled fork can therefore run modified application code with both credentials and exfiltrate them. The preview-deploy label authorizes execution but does not isolate PR-controlled code.

Move untrusted builds to a no-secret job. Deploy only immutable image digests with a trusted chart and least-privilege preview credentials.

🧰 Tools
🪛 zizmor (1.29.0)

[error] 20-38: use of fundamentally insecure workflow trigger (dangerous-triggers): pull_request_target is almost always used insecurely

(dangerous-triggers)

🤖 Prompt for AI Agents 
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In @.github/workflows/preview-deploy.yml around lines 20 - 22, Restructure the
preview workflow so pull-request-controlled code is built only in a job without
secrets, while the deployment job uses a trusted chart and immutable image
digests rather than checking out or executing the PR head. Ensure the
secret-bearing deployment receives only least-privilege preview credentials, and
retain the existing preview-deploy label authorization without allowing
fork-controlled Dockerfiles, charts, or application code to access secrets.

Source: Linters/SAST tools

branches: [main, dev]
workflow_dispatch:
inputs:
Expand All @@ -27,6 +31,11 @@ on:
description: "Git ref to build (branch, tag, or SHA). Defaults to the branch the workflow is dispatched from."
required: false
default: ""
public:
description: "Expose the preview without the oauth2-proxy login gate. Anyone with the URL can reach it."
type: boolean
required: false
default: false

concurrency:
group: preview-${{ github.event.pull_request.number || inputs.preview_id }}
Expand Down Expand Up @@ -73,10 +82,17 @@ jobs:
# PRs (head.repo == base repo) always run; fork PRs require the
# `preview-deploy` label. Manual workflow_dispatch is always allowed
# (maintainer-initiated).
#
# `unlabeled` is in the trigger list only so that removing
# `preview-public` redeploys with the auth gate back on. Every other
# label removal is filtered out here — otherwise unrelated label
# churn would kick off a full rebuild.
if: |
github.event_name == 'workflow_dispatch' ||
github.event.pull_request.head.repo.full_name == github.repository ||
contains(github.event.pull_request.labels.*.name, 'preview-deploy')
((github.event.action != 'unlabeled' || github.event.label.name == 'preview-public') && (
github.event.pull_request.head.repo.full_name == github.repository ||
contains(github.event.pull_request.labels.*.name, 'preview-deploy')
))
# Org-level self-hosted runner in arc-runners-ndif-team. The
# ramdisk variant mounts a 32 GiB tmpfs at the runner workspace so
# bun install / uv sync / next build / docker layer extraction run
Expand All @@ -95,14 +111,21 @@ jobs:
# SHA is fetchable in the base-repo context that
# pull_request_target runs in.
ref="${{ github.event.pull_request.head.sha }}"
public="${{ contains(github.event.pull_request.labels.*.name, 'preview-public') }}"
else
preview_id="${{ inputs.preview_id }}"
ref="${{ inputs.ref }}"
[ -z "$ref" ] && ref="${{ github.ref_name }}"
public="${{ inputs.public }}"
fi
# The chart's ingress.authGate.enabled is the inverse: public
# preview => no gate.
auth_gate=true
[ "$public" = "true" ] && auth_gate=false
{
echo "preview_id=${preview_id}"
echo "ref=${ref}"
echo "auth_gate=${auth_gate}"
} | tee -a "$GITHUB_OUTPUT"

# Explicitly check out the PR head — pull_request_target defaults
Expand Down Expand Up @@ -286,6 +309,7 @@ jobs:
HOST: ${{ steps.meta.outputs.host }}
API_HOST: ${{ steps.meta.outputs.api_host }}
TAG: ${{ steps.meta.outputs.tag }}
AUTH_GATE: ${{ steps.pre.outputs.auth_gate }}
run: |
# --atomic: if the upgrade fails before --timeout, helm rolls
# back to the prior revision in the same shell invocation,
Expand All @@ -305,6 +329,7 @@ jobs:
--set commitSha="${SHA}" \
--set ingress.host="${HOST}" \
--set ingress.className="${INGRESS_CLASS}" \
--set ingress.authGate.enabled="${AUTH_GATE}" \
--atomic --cleanup-on-fail \
--wait --timeout 20m

Expand All @@ -318,6 +343,11 @@ jobs:
echo "- Web image: \`${{ env.IMAGE_NAME }}:${{ steps.meta.outputs.tag }}\`"
echo "- API image: \`${{ env.IMAGE_NAME_API }}:${{ steps.meta.outputs.tag }}\`"
echo "- Namespace: \`${{ steps.meta.outputs.namespace }}\`"
if [ "${{ steps.pre.outputs.auth_gate }}" = "false" ]; then
echo "- Access: **public** (no login gate — \`preview-public\`)"
else
echo "- Access: ndif-team GitHub login required"
fi
Comment on lines +346 to +350

@coderabbitai coderabbitai Bot Aug 10, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

Do not attribute manual public deployments to the PR label.

When workflow_dispatch sets public: true, no preview-public label exists. The summary still reports that label as the reason for public access. Remove the label reference or emit a separate access-source output.

Proposed fix 
-              echo "- Access: **public** (no login gate — \`preview-public\`)"
+              echo "- Access: **public** (no login gate)"
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
if [ "${{ steps.pre.outputs.auth_gate }}" = "false" ]; then
echo "- Access: **public** (no login gate — \`preview-public\`)"
else
echo "- Access: ndif-team GitHub login required"
fi
if [ "${{ steps.pre.outputs.auth_gate }}" = "false" ]; then
echo "- Access: **public** (no login gate)"
else
echo "- Access: ndif-team GitHub login required"
fi
🧰 Tools
🪛 zizmor (1.29.0)

[info] 346-346: code injection via template expansion (template-injection): may expand into attacker-controllable code

(template-injection)

🤖 Prompt for AI Agents 
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In @.github/workflows/preview-deploy.yml around lines 346 - 350, Update the
access summary conditional around steps.pre.outputs.auth_gate so manually
dispatched deployments with public=true are not described as using the
preview-public label. Remove the label attribution from the public message, or
add and use a distinct access-source output that differentiates manual public
deployment from label-based access.

} >> "$GITHUB_STEP_SUMMARY"

- name: Comment preview URL on PR
Expand All @@ -333,3 +363,4 @@ jobs:
- Web image: `${{ env.IMAGE_NAME }}:${{ steps.meta.outputs.tag }}`
- API image: `${{ env.IMAGE_NAME_API }}:${{ steps.meta.outputs.tag }}`
- Namespace: `${{ steps.meta.outputs.namespace }}`
- Access: ${{ steps.pre.outputs.auth_gate == 'false' && '🌐 **public** — anyone with the URL can reach this preview (`preview-public` label). Remove the label to re-gate.' || '🔒 ndif-team GitHub login required' }}
7 changes: 7 additions & 0 deletions deploy/preview/values.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,13 @@ ingress:
# See k8s/apps/workbench-preview-auth/ in the cluster repo. Set
# enabled=false to expose a preview publicly (e.g. for an external
# collaborator who can't be added to the org).
#
# On CI this is driven by the `preview-public` PR label (or the
# `public` input on a workflow_dispatch run) — see the `auth_gate`
# output in .github/workflows/preview-deploy.yml. Note that a public
# preview also exposes /admin (the ADMIN_EMAILS allowlist below
# matches the stub user every anonymous visitor gets) and lets anyone
# drive NDIF inference through this deploy's NDIF_API_KEY.
authGate:
enabled: true
host: auth.ndif-preview.ripley.cloud
Expand Down
Loading