ci(preview): add a preview-public label to drop the oauth2 gate - #138
ci(preview): add a preview-public label to drop the oauth2 gate#138jon-bell wants to merge 1 commit into
Conversation
Preview deploys sit behind oauth2-proxy (GitHub, ndif-team org-only), which is the right default but blocks the case the gate was always going to run into: showing a preview to an external collaborator who can't be added to the org. deploy/preview already had the switch (ingress.authGate.enabled); the workflow just never passed it, so previews were gated unconditionally. Wire it to a `preview-public` PR label, plus a `public` input for workflow_dispatch runs. `unlabeled` joins the trigger list so that removing the label redeploys with the gate back on. Every other label removal is filtered out in the job's `if:` — otherwise unrelated label churn would kick off a full rebuild. The sticky PR comment and job summary now state which of the two a preview is, since "this URL is open to the world" shouldn't be something you have to read the ingress annotations to discover. Worth knowing before reaching for the label: the preview image bakes NEXT_PUBLIC_DISABLE_AUTH=true, so every anonymous visitor is the stub user dev@localhost, which the ADMIN_EMAILS allowlist matches — a public preview has a public /admin. The api pod also holds NDIF_API_KEY. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
📝 WalkthroughWalkthroughThe preview workflow now supports public previews through the ChangesPreview authentication control
Estimated code review effort: 2 (Simple) | ~10 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
The latest updates on your projects. Learn more about Argos notifications ↗︎
|
|
🚀 Preview deployed
|
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with 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.
Inline comments:
In @.github/workflows/preview-deploy.yml:
- Around line 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.
- Around line 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.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 0e965e4f-e0ab-4c3f-86e3-d2d7b1b2e61e
📒 Files selected for processing (2)
.github/workflows/preview-deploy.ymldeploy/preview/values.yaml
| on: | ||
| pull_request_target: | ||
| types: [opened, reopened, synchronize, labeled] | ||
| types: [opened, reopened, synchronize, labeled, unlabeled] |
There was a problem hiding this comment.
🔒 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.ymlRepository: 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
| 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 |
There was a problem hiding this comment.
🎯 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.
| 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.
Preview deploys sit behind oauth2-proxy (GitHub, ndif-team org-only). That's the right default, but it blocks the case the gate was always going to run into: showing a preview to an external collaborator who can't be added to the org.
deploy/previewalready had the switch (ingress.authGate.enabled) — the workflow just never passed it, so previews were gated unconditionally. This wires it up.How to use it
preview-publiclabel. The next deploy drops the gate; removing the label redeploys with it back on.workflow_dispatchgains apublicboolean input.unlabeledjoins the trigger list so label removal actually re-gates. Every other label removal is filtered out in the job'sif:— otherwise unrelated label churn would kick off a full rebuild.The sticky PR comment and job summary now report which mode a preview is in. "This URL is open to the world" shouldn't be something you have to read the ingress annotations to discover.
Before reaching for the label
A public preview is more open than it first looks:
NEXT_PUBLIC_DISABLE_AUTH=true, so every anonymous visitor is the stub userdev@localhost— which is exactly whatADMIN_EMAILSinvalues.yamlallowlists. So a public preview has a public, writable/admin, and sinceNEXT_PUBLIC_LOCAL_DB=trueis server-side SQLite in the pod, one visitor's tutorial/workshop edits are visible to all of them.NDIF_API_KEYandHF_TOKEN. A public api host means anyone with the URL can drive NDIF inference on that key.Both are noted in a comment on
values.yaml. Neither is new — they were just previously covered by the gate. If public previews become routine rather than occasional, droppingADMIN_EMAILSwhen the gate is off is the obvious follow-up.Testing
helm templatewithauthGate.enabledfalse/true renders 0 / 2auth-urlannotations respectively.helm upgrade --set ingress.authGate.enabled=falsewas applied by hand to the PR more J lens #136 preview; both ingresses lost their forward-auth annotations and the app +/docsserve 200 with no login.🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Documentation