Skip to content

ci(ui-preview): give fork PRs preview deploys (build-untrusted / deploy-trusted split) - #643

Merged
JSONbored merged 2 commits into
mainfrom
fix/gate-confirmed-contributor
Jun 13, 2026
Merged

ci(ui-preview): give fork PRs preview deploys (build-untrusted / deploy-trusted split)#643
JSONbored merged 2 commits into
mainfrom
fix/gate-confirmed-contributor

Conversation

@JSONbored

@JSONbored JSONbored commented Jun 13, 2026

Copy link
Copy Markdown
Owner

Problem

Fork PRs (e.g. #635) never get a UI preview, so reviewbot's before/after comment is stuck forever on the "Rendering preview…" placeholder.

Root cause: ui-preview.yml is gated if: head.repo.full_name == github.repository — forks have no Cloudflare secrets, so the job skips entirely → no Deployment recorded → reviewbot has no "after" URL. Workers Builds can't cover it either: it only builds branches in this repo, and a fork PR's commits live in the contributor's fork. So neither mechanism builds fork code.

Fix — split build (untrusted) from deploy (trusted)

  • ui-preview.yml (renamed UI Preview Build, on: pull_request): builds every PR incl forks, without secrets (read-only token), and uploads only the built dist artifact.
  • ui-preview-deploy.yml (new, on: workflow_run): runs from the default branch (trusted definition, never the fork's) with secrets. Downloads the built artifact, validates it (no symlinks, expected SSR structure, allowlisted file types), wrangler versions uploads it to a transient workers.dev preview, and records the preview/pr-N GitHub Deployment + status reviewbot already reads.

Security model

  • Build runs fork code with zero secret access.
  • Deploy has secrets but runs no fork codewrangler only uploads the bundle; fork code executes solely inside the isolated workers.dev preview when the URL is visited.
  • The deploy job is bound to a preview environment so approval gating / scoped secrets can be attached.
  • Downloaded fork artifact is validated before deploy (rejects symlinks, enforces structure + file-type allowlist).
  • PR number resolved from the GitHub-set head_sha (workflow_run.pull_requests is empty for forks), never fork-supplied data; the Wrangler config is written trust-side so a fork can't control bindings/routes/vars.

No linked issue: maintainer-lane CI chore

No linked issue: this is a maintainer-lane CI / infra chore on the PR-preview pipeline (safely enabling fork-PR previews). There is no separate tracking issue — it was surfaced directly while debugging reviewbot's stuck "Rendering preview…" on fork PR #635 (referenced for context).

Notes

  • No reviewbot change — it reads the same preview/pr-N Deployment as before.
  • workflow_run always uses the default-branch workflow definition, so the deploy half can't fully run on this PR — it takes effect once merged. Validate afterward on a fork PR (e.g. update feat(api): maintainer quality-dashboard service + endpoint #635 with main so its build re-runs).
  • Verified with actionlint (clean).

🤖 Generated with Claude Code

@ghost

ghost commented Jun 13, 2026

Copy link
Copy Markdown

Note

Gittensory Gate skipped

PR closed before full evaluation. No late first comment was created.

Signal Result Evidence Action
Gate result ⚠️ Skipped #643 is no longer open. No action.

💰 Earn for open-source contributions like this. Gittensor lets GitHub contributors earn for the work they already do — register to start earning →.

Checked by Gittensory, a quiet PR intelligence layer for OSS maintainers.

@ghost ghost added the gittensory:reviewed label Jun 13, 2026
@ghost

ghost commented Jun 13, 2026

Copy link
Copy Markdown

gittensory · advisory review

Reviewed 2 changed file(s) — two independent AI reviewers.

Suggested action:Safe to merge — both reviewers found no blocking issues.

Reviewer A · gpt-oss-120b — recommends ✅ merge
This PR splits the UI preview pipeline into a trusted deploy workflow and a build‑only workflow, adding robust validation of the built artifact and ensuring secrets are only used in the trusted step. The changes improve security for fork PRs without altering functionality.

Suggestions

  • Verify that the environment.url reference to steps.upload.outputs.preview_url resolves correctly, as environment URLs are evaluated after steps complete.
  • Consider adding a short comment explaining the purpose of the concurrency group naming to aid future maintainers.

Worth double-checking

  • If the artifact validation regex misses a dangerous file type, a malicious bundle could be deployed.
  • The deploy workflow assumes the wrangler upload output contains a workers.dev URL with 'gittensory-ui'; any change in wrangler output format could break URL extraction.

Reviewer B · mistral-small-3.1-24b-instruct — recommends ✅ merge
This PR splits the UI preview pipeline into build and deploy stages, allowing fork PRs to receive preview deploys. The changes are well-documented and follow best practices for security and concurrency.

Suggestions

  • Consider adding a step to validate the VITE_GITTENSORY_API_ORIGIN environment variable to ensure it is correctly set and secure.
  • Add a comment explaining why the wrangler version is pinned to 4.95.0 to ensure future maintainers understand the rationale.

Worth double-checking

  • Ensure that the artifact validation step correctly handles all edge cases, especially with unexpected file types.
  • Verify that the deployment process correctly handles concurrent builds and deploys to avoid conflicts.

@ghost ghost added the gittensory-review label Jun 13, 2026

@superagent-security superagent-security Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Superagent found 2 security concern(s).

Comment thread .github/workflows/ui-preview-deploy.yml
Comment thread .github/workflows/ui-preview-deploy.yml
@superagent-security superagent-security Bot added the pr:flagged PR flagged for review by security analysis. label Jun 13, 2026
@JSONbored JSONbored self-assigned this Jun 13, 2026
@superagent-security

Copy link
Copy Markdown
Contributor

Superagent didn't find any vulnerabilities or security issues in this PR.

@superagent-security superagent-security Bot removed the pr:flagged PR flagged for review by security analysis. label Jun 13, 2026
… deploy-trusted split

Fork PRs never got a UI preview: ui-preview.yml was gated `if: head.repo.full_name
== github.repository` (forks have no Cloudflare secrets), so it skipped entirely,
no Deployment was recorded, and Reviewbot's before/after comment was stuck on a
perpetual "Rendering preview…" placeholder. Workers Builds can't fill the gap
either — it only builds branches that live in this repo, and a fork PR's commits
live in the contributor's fork.

Split the pipeline so forks get previews without exposing secrets to untrusted code:
- ui-preview.yml (renamed "UI Preview Build", on: pull_request): builds EVERY PR
  incl forks, WITHOUT secrets (read-only token), and uploads only the built `dist`.
- ui-preview-deploy.yml (new, on: workflow_run): runs from the DEFAULT branch (so the
  definition is trusted, never the fork's) WITH secrets. Downloads the built artifact
  (never checks out or runs fork source), `wrangler versions upload`s it to a transient
  workers.dev preview, and records the GitHub Deployment + status Reviewbot reads.

Security boundary: the build runs fork code with no secret access; the deploy has
secrets but runs no fork code (wrangler only uploads the bundle — fork code executes
solely inside the isolated workers.dev preview when the URL is visited). The PR number
is resolved from the GitHub-set head_sha (workflow_run.pull_requests is empty for forks),
never from fork-supplied data, and the Wrangler config is written trustside so a fork
can't control bindings/routes/vars.

No Reviewbot change needed — it reads the same `preview/pr-N` Deployment as before.
Note: the deploy half only takes effect once this is on the default branch (workflow_run
always uses the default-branch definition), so it can't fully run on this PR itself.
Address the security findings + reviewer notes on the fork-preview deploy workflow:
- Bind the deploy job (the only one with Cloudflare secrets) to a `preview`
  GitHub environment so approval gating / environment-scoped secrets can be
  attached. Unprotected by default so previews stay automatic. (Superagent P1)
- Validate the untrusted fork-built artifact before handing it to wrangler:
  reject symlinks (path-traversal/exfil vector), require the expected SSR
  structure (server/index.mjs + client/), and allowlist file extensions so a
  malicious build can't smuggle scripts/binaries. (Superagent P1)
- Pin the parsed preview URL to the gittensory-ui worker on workers.dev, so a
  changed wrangler output format or any other URL in the logs can't be recorded
  as the preview by mistake. (both reviewers)

actionlint (incl shellcheck) clean.
@JSONbored
JSONbored force-pushed the fix/gate-confirmed-contributor branch from ced305f to 82b12cc Compare June 13, 2026 08:38
@JSONbored
JSONbored merged commit e86c082 into main Jun 13, 2026
10 checks passed
@JSONbored
JSONbored deleted the fix/gate-confirmed-contributor branch June 13, 2026 08:52
@github-project-automation github-project-automation Bot moved this from Todo to Done in gittensory - v1 roadmap Jun 13, 2026
JSONbored added a commit that referenced this pull request Jun 13, 2026
… config (#645)

The trusted preview config embedded the production custom domain
(routes: gittensory.aethereal.dev, custom_domain: true). It is inert under
`wrangler versions upload` (a 0%-traffic preview version applies no routes),
but it was a latent footgun: if that command were ever changed to
`wrangler deploy`, fork-built code would immediately seize the production
domain at 100% traffic. A workers.dev preview neither needs nor uses the
route, so removing it eliminates that path; a comment now warns against
re-adding it.

Defense-in-depth follow-up to the fork-safe preview pipeline (#643), flagged
by an adversarial security audit of that pipeline.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

No open projects
Status: Done

Development

Successfully merging this pull request may close these issues.

1 participant