deckhearth/.convoys/fix-vercel-deployment-protection-in-ci/brief-1-inject-bypass-secret-into-ci-workflows.md

---
convoy: fix-vercel-deployment-protection-in-ci
brief_number: 1
depends_on: []
files:
  - .github/workflows/preview-smoke.yml
  - .github/workflows/visual-diff.yml
---

# Brief 1: Inject `VERCEL_AUTOMATION_BYPASS_SECRET` into the preview-smoke + visual-diff workflows so the wait-for-vercel-preview healthcheck passes against protected Vercel previews

## Goal (1 sentence)

Plumb `${{ secrets.VERCEL_AUTOMATION_BYPASS_SECRET }}` into `wait-for-vercel-preview`'s `path:` input as a query parameter (Decision A) in both workflows, tighten `max_timeout` from 600 → 120 (Decision B), extend the `gate:` job to skip fork PRs (Decision D), and forward the same secret as an env var to the Playwright smoke / screenshot-capture step so `adopt-playwright-smoke` finds the surface pre-wired — without ever leaking the secret to a workflow log or to `${{ steps.vercel.outputs.url }}`.

## Files in scope (do not edit anything else)

- `.github/workflows/preview-smoke.yml` — modified.
- `.github/workflows/visual-diff.yml` — modified.

**Files explicitly out of scope** (do not touch even if it seems related):

- `playwright.config.js` — does not exist yet; `adopt-playwright-smoke` owns it.
- `tests/smoke/app.smoke.spec.ts` — already exists as a stub but stays in `tests/smoke/`; `adopt-playwright-smoke` owns it.
- `tests/visual/` — does not exist yet; `adopt-playwright-smoke` owns it.
- `AGENTS.md` § 7 — the wording correction (header → query param; queued → wired) is the doc-writer pass at convoy close, NOT this brief.
- `package.json` — no new deps. `@playwright/test` is not in scope here.
- Any other workflow in `.github/workflows/` (e.g. `ci.yml`) — out of scope.

## Conventions to follow

- **Decision A (`.convoys/fix-vercel-deployment-protection-in-ci.md` § Decisions post-IA round).** The wait-action receives the bypass via query param on `path:`. The header form is reserved for the future `playwright.config.js`.
- **Decision B (same file).** `max_timeout: 600` → `max_timeout: 120` in both workflows. Operator may override at gate 1 if Vercel builds have been slow recently.
- **Decision C (same file).** Do NOT place `${{ secrets.* }}` inside any `concurrency:` group expression. GitHub Actions YAML parser rejects secret refs in `concurrency:` and the workflow fails to load. The `concurrency:` blocks stay unchanged.
- **Decision D (same file).** Extend the existing `gate:` step's `Decide` shell script to check `github.event.pull_request.head.repo.fork` FIRST. When the PR comes from a fork, emit `::notice::` and set `should_run=false`. Same shape in both workflows.
- **No-go zones (`.cursor/rules/no-go-zones.mdc`).** None of the files in scope are in the no-go list. Do not edit anything outside `.github/workflows/preview-smoke.yml` and `.github/workflows/visual-diff.yml` in this brief.
- **Secret-handling discipline:**
  - NEVER `echo`, `cat`, or `printf` a URL or env var that contains `${{ secrets.VERCEL_AUTOMATION_BYPASS_SECRET }}` or `$VERCEL_AUTOMATION_BYPASS_SECRET`.
  - NEVER use `set -x` in any `run:` step in either workflow (it would echo every command including ones that interpolate secrets).
  - NEVER assign the constructed URL (the one with the bypass query) to a shell variable that is later printed.
  - The wait-action's own logs are safe (see Boot-the-brief finding #2 below). The risk is in YOUR additions, not in the action.
  - GitHub Actions auto-masks values that match registered secrets in workflow logs. That is a backstop, not a primary defense. Do not rely on it to redact full URLs.
- **Style match.** PR #16 (`fix(ci): scoped permissions for preview-smoke + visual-diff workflows`, squash commit `7e97254`) is the precedent for touching both workflow files in the same PR. Follow its diff shape: same change applied to both files, with workflow-specific differences (smoke vs visual-diff naming, the `pull-requests: write` permission only on visual-diff) preserved as-is.

## Acceptance criteria

### `.github/workflows/preview-smoke.yml`

- [ ] **Wait-action step (currently lines 60-65) becomes:**

```yaml
      - name: Wait for Vercel Preview deployment
        id: vercel
        uses: patrickedqvist/wait-for-vercel-preview@v1.3.2
        with:
          token: ${{ secrets.GITHUB_TOKEN }}
          max_timeout: 120
          path: /?x-vercel-protection-bypass=${{ secrets.VERCEL_AUTOMATION_BYPASS_SECRET }}&x-vercel-set-bypass-cookie=true
```

  Three changes relative to the current step:

  1. `max_timeout: 600` → `max_timeout: 120` (Decision B).
  2. New `path:` input — **MUST begin with a leading `/`**. `action.js:42` parses this via `new URL(path, url)`; a missing leading `/` makes the URL resolver produce a path relative to the wrong base. Verbatim `/` then `?` then the two query params.
  3. The `x-vercel-set-bypass-cookie=true` segment is REQUIRED, not optional. It causes Vercel to set a `_vercel_jwt` cookie on the response so any follow-up same-origin requests (e.g. Playwright's `page.goto` redirects) reuse the bypass without needing the query string again.

  Do NOT:
  - Move the `path:` line above `max_timeout:` (no semantic difference, but match the verbatim order so the diff stays minimal).
  - Use single quotes around the `path:` value. YAML treats the unquoted form as a plain string; quoting introduces escape-handling questions. Leave it unquoted.
  - Add an `env:` block to this step. The wait-action does not read `process.env.VERCEL_AUTOMATION_BYPASS_SECRET`; it consumes the input only.

- [ ] **`gate:` job's `Decide` step (currently lines 41-49) becomes:**

```yaml
      - name: Decide
        id: check
        run: |
          if [[ "${{ github.event.pull_request.head.repo.fork }}" == "true" ]]; then
            echo "should_run=false" >> $GITHUB_OUTPUT
            echo "::notice::Smoke skipped on fork PR (bypass secret unavailable to forks)"
          elif echo "${{ github.event.pull_request.body }}" | grep -qE 'pipeline:.*skip.*\bsmoke\b'; then
            echo "should_run=false" >> $GITHUB_OUTPUT
            echo "::notice::Smoke skipped via pipeline directive"
          else
            echo "should_run=true" >> $GITHUB_OUTPUT
          fi          
```

  Three changes relative to current:

  1. New `if [[ "${{ ... fork }}" == "true" ]]` branch FIRST. It must come before the body-directive check so fork PRs short-circuit out without parsing the PR body.
  2. The shell comparison is `== "true"` (a string compare against the literal string `"true"`). `github.event.pull_request.head.repo.fork` is rendered as the string `"true"` or `"false"` in expression context — NOT as a bare boolean (Risk R5 in the convoy's architecture risk list).
  3. The notice mentions "fork PR" explicitly; this surfaces in the GitHub Actions UI summary so a reader scanning a PR can immediately see why smoke didn't run.

  Do NOT:
  - Replace the body-directive check (`pipeline:.*skip.*\bsmoke\b`). That gate is still useful for non-fork PRs that legitimately want to skip smoke (e.g. doc-only PRs).
  - Move the `gate:` step's `if:` higher — `if: needs.gate.outputs.should_run == 'true'` on the `smoke:` job is the correct gate; it stays.

- [ ] **Playwright smoke step (currently lines 77-80) becomes:**

```yaml
      - name: Run smoke tests
        run: npx playwright test --project=smoke
        env:
          BASE_URL: ${{ steps.vercel.outputs.url }}
          VERCEL_AUTOMATION_BYPASS_SECRET: ${{ secrets.VERCEL_AUTOMATION_BYPASS_SECRET }}
```

  One change relative to current:

  1. New `VERCEL_AUTOMATION_BYPASS_SECRET: ${{ secrets.VERCEL_AUTOMATION_BYPASS_SECRET }}` line in the step's `env:` block. This is forward-compat plumbing — the `playwright.config.js` that `adopt-playwright-smoke` ships will read `process.env.VERCEL_AUTOMATION_BYPASS_SECRET` and inject the bypass via `extraHTTPHeaders` per Vercel's snippet.
  2. **Do NOT modify `BASE_URL`.** It stays `${{ steps.vercel.outputs.url }}`. The wait-action's `outputs.url` is the bare deployment URL — the bypass query string is NOT appended (confirmed by reading `action.js:360`). Playwright will inject the bypass via headers; the URL must stay clean so the headers actually apply on every request (Playwright re-applies `extraHTTPHeaders` per request, including redirects).
  3. Until `adopt-playwright-smoke` ships, this step will FAIL because `@playwright/test` is not installed. That's the documented end state of THIS brief (see acceptance criterion #1 at the bottom of this section); do not try to fix it here.

- [ ] **All other lines in `.github/workflows/preview-smoke.yml` stay byte-for-byte identical to the current file** — including:
  - The `name:` line.
  - The full `on:` block (PR types, target branch).
  - The full `concurrency:` block (Decision C — no secret reference).
  - The full `permissions:` block (PR #16 already landed the minimal scope; do not touch).
  - The `gate:` job's `name:`, `runs-on:`, `outputs:`, the existing `actions/checkout@v4` step in the `smoke:` job, `actions/setup-node@v4`, `npm ci`, `npx playwright install --with-deps chromium`, and the `Upload Playwright report on failure` step.
  - The leading multi-line comment block at the top of the file (lines 1-11). Update text is the doc-writer's job, not the implementer's.

### `.github/workflows/visual-diff.yml`

- [ ] **Wait-action step (currently lines 56-61) becomes:**

```yaml
      - name: Wait for Vercel Preview deployment
        id: vercel
        uses: patrickedqvist/wait-for-vercel-preview@v1.3.2
        with:
          token: ${{ secrets.GITHUB_TOKEN }}
          max_timeout: 120
          path: /?x-vercel-protection-bypass=${{ secrets.VERCEL_AUTOMATION_BYPASS_SECRET }}&x-vercel-set-bypass-cookie=true
```

  Identical to the preview-smoke version (same wait-action, same input shape, same secret). The shared shape is intentional — Decision A applies to both workflows.

- [ ] **`gate:` job's `Decide` step (currently lines 38-45) becomes:**

```yaml
      - id: check
        run: |
          if [[ "${{ github.event.pull_request.head.repo.fork }}" == "true" ]]; then
            echo "should_run=false" >> $GITHUB_OUTPUT
            echo "::notice::Visual diff skipped on fork PR (bypass secret unavailable to forks)"
          elif echo "${{ github.event.pull_request.body }}" | grep -qE 'pipeline:.*skip.*\bvisual\b'; then
            echo "should_run=false" >> $GITHUB_OUTPUT
            echo "::notice::Visual diff skipped via pipeline directive"
          else
            echo "should_run=true" >> $GITHUB_OUTPUT
          fi          
```

  Same shape as preview-smoke's gate, with two text-only differences:

  1. The body-directive regex is `\bvisual\b` (was `\bsmoke\b` in preview-smoke). Matches the existing convention in the current file.
  2. The two `::notice::` strings say "Visual diff" instead of "Smoke" — matches the workflow's name.

  Note: the current `visual-diff.yml` `Decide` step is missing the `name:` field (the current file is `- id: check` directly). Preserve that style — do not add a `name:` here just because preview-smoke has one. The diff stays minimal.

- [ ] **Screenshot-capture step (currently lines 71-75) becomes:**

```yaml
      - name: Capture screenshots (PR)
        run: npx playwright test --project=visual --update-snapshots=none
        env:
          BASE_URL: ${{ steps.vercel.outputs.url }}
          VERCEL_AUTOMATION_BYPASS_SECRET: ${{ secrets.VERCEL_AUTOMATION_BYPASS_SECRET }}
        continue-on-error: true
```

  Same change as preview-smoke's smoke step — add `VERCEL_AUTOMATION_BYPASS_SECRET` to `env:`. Do NOT remove `continue-on-error: true` (the visual-diff workflow's design is to upload diffs even when tests fail; that behavior stays).

- [ ] **All other lines stay byte-for-byte identical** — including:
  - `name:`, `on:` (paths-only trigger), `concurrency:`, `permissions:` (note: visual-diff has `pull-requests: write` because of the comment-on-PR step; do not change this).
  - The leading multi-line comment block at the top of the file (lines 1-5).
  - The `actions/upload-artifact@v4` and `actions/github-script@v7` (comment-on-PR) steps.

### Cross-file checks (apply to both YAML files)

- [ ] **No `${{ secrets.* }}` reference appears inside the `concurrency:` block** in either file. Grep before committing: `rg 'secrets\.' .github/workflows/preview-smoke.yml .github/workflows/visual-diff.yml` should return exactly two matches per file (one in the wait-action's `path:`, one in the Playwright/visual step's `env:`). Three or more matches per file means a stray secret reference snuck somewhere; investigate.
- [ ] **No `echo`, `printf`, `cat`, or `set -x` references the bypass-bearing URL or env var.** Grep: `rg 'echo|printf|cat|set -x' .github/workflows/preview-smoke.yml .github/workflows/visual-diff.yml` — every match should be an existing line untouched by this brief (the `gate:` job's `echo "should_run=..." >> $GITHUB_OUTPUT` lines are fine; they don't echo URLs or secrets).
- [ ] **Diff hygiene.** `git diff main..HEAD -- .github/workflows/` should show only the changes specified above. No whitespace-only changes elsewhere. No unrelated edits. Total diff is expected to be ~50 LOC across both files (~20 LOC per file changed, plus a few context lines).

### Acceptance criterion #1 — end-state behavior

After this brief lands on the convoy branch and a Vercel preview deployment is published for the PR:

- [ ] **The wait-action's healthcheck exits successfully.** In the `Playwright smoke` job's run log, expect a single line near the end of the `Wait for Vercel Preview deployment` step:
  ```
  Received success status code
  ```
  This means the bypass query made it through to Vercel and the protected preview returned 2xx.
- [ ] **The wait-action step takes < 90 seconds wall time** (typical: < 10s if the deployment is already up, which is the usual case based on PR #16's logs).
- [ ] **The subsequent `Run smoke tests` step (preview-smoke) or `Capture screenshots (PR)` step (visual-diff) is REACHED**, even though it will fail because `@playwright/test` is not installed. The expected failure mode is roughly:
  ```
  npm ERR! could not determine executable to run
  npm error code ENOENT
  npm error path .../node_modules/@playwright/test
  ```
  OR a `npx`-driven download that succeeds but then fails on the missing config. Either is acceptable for this brief — the success metric is "wait-action passed and Playwright step was reached," not "Playwright passed."
- [ ] **Total job runtime is < 5 minutes.** The convoy file's success metric. Now-correctly-passing wait step (~10s) + reached-but-failing Playwright step (~30-90s) is well inside 5 minutes.
- [ ] **The bypass secret value does NOT appear in any line of the run log.** Verify after the run:
  ```bash
  gh run download <run-id> -n logs
  # Then locally — NEVER commit this script — check that the secret's
  # first 8 chars do not appear in the downloaded logs. GitHub Actions
  # also auto-masks; this is belt-and-suspenders.
  rg "$(head -c 8 <<< "$VERCEL_AUTOMATION_BYPASS_SECRET")" logs/ || echo "OK — bypass not in logs"
  ```
  Expected: `OK — bypass not in logs`.

## Manual verification (in addition to the workflow run on push)

Run these in order. Paste relevant output (with secrets redacted) into the PR description.

- [ ] **Local YAML lint.** Install actionlint (one-time):
  ```bash
  brew install actionlint
  ```
  Then validate:
  ```bash
  actionlint .github/workflows/preview-smoke.yml .github/workflows/visual-diff.yml
  ```
  Expected: zero output, exit code 0. If actionlint flags anything,
  read the message — most actionlint warnings are real (shellcheck
  embedded). Investigate before commit. If `brew` is unavailable, the
  binary is downloadable from the actionlint releases page; recommended
  but not strictly required by acceptance criterion (the workflow YAML
  is small enough to eyeball).

- [ ] **Local `gh` dry-run check.** Verify the secret is still seeded
  (operator says it is, but confirm before pushing):
  ```bash
  gh secret list | grep VERCEL_AUTOMATION_BYPASS_SECRET
  ```
  Expected: one line showing the secret name and an `Updated` timestamp.

- [ ] **Push the branch and observe the first workflow run.** From the
  convoy branch (`convoy/fix-vercel-deployment-protection-in-ci`):
  ```bash
  git push -u origin HEAD
  ```
  Then watch the Preview-smoke and Visual-diff workflows. The first
  Vercel preview deploy on this PR is the one to scrutinize. Expect:
  - Wait-action step logs `target url » https://<deployment>.vercel.app`
    (note: no query string in this log — that's the action's safe log
    of the bare deployment URL).
  - Within a few seconds, `Received success status code`.
  - Step exits 0.
  - Next step (`Set up Node`, `npm ci`, etc.) runs.
  - Eventually fails at the Playwright step — that's the expected end
    state of THIS brief.

- [ ] **Re-run validation.** Click "Re-run jobs" on the same run. Expect
  identical behavior — the wait-action's healthcheck issues fresh
  axios GETs on every iteration (no caching), so re-runs are
  idempotent (Risk R-not-listed-because-confirmed-OK).

- [ ] **Bypass-log-leak validation** (as documented above under acceptance
  criterion #1, final bullet).

## Boot-the-brief findings (preempted by the architect; do not re-investigate)

### Finding 1 — Wait-action source: `path:` is consumed via `new URL(path, url)`

`patrickedqvist/wait-for-vercel-preview@v1.3.2` at `action.js:42`:

```js
let checkUri = new URL(path, url);
await axios.get(checkUri.toString(), { headers });
```

Parsed as a URL relative to `url` (the deployment URL). Query strings
work verbatim. **MUST begin with `/`** or the URL resolver produces
unexpected paths.

### Finding 2 — Wait-action source: bypass secret never appears in the action's logs

Action source emits exactly three `console.log` calls that include URL
content:

1. `action.js:357` — `console.log('target url »', targetUrl)`. `targetUrl`
   is `status.target_url` (the bare deployment URL from the GitHub
   Deployments API). **No `path:` is appended.** ✅ Safe.
2. `action.js:363` — `console.log('Waiting for a status code 200 from: ${targetUrl}')`. Same `targetUrl`. ✅ Safe.
3. `action.js:53-54` — `console.log('GET status: ${e.response.status}. Attempt ${i} of ${iterations}')`. Only the HTTP status code, no URL. ✅ Safe.

The bypass query string lives ONLY in the internal `checkUri` axios
call. **This means: passing the bypass via `path:` is structurally
safe from log leakage by the action itself.** Your remaining job is to
not add any echo/print step in the workflow YAML that constructs a URL
with the bypass.

### Finding 3 — `outputs.url` is the bare URL (no bypass query)

`action.js:360`: `core.setOutput('url', targetUrl)` — `targetUrl` does
NOT include `path:`. So `${{ steps.vercel.outputs.url }}` downstream is
clean. **This is why the Playwright `BASE_URL` env var stays bare:** the
future `playwright.config.js` will inject the bypass via
`extraHTTPHeaders`, NOT by reconstructing a URL with the query string.

### Finding 4 — Empty/unset secret on fork PRs

GitHub Actions silently omits repo secrets on `pull_request`-event runs
from forks. If the fork-PR gate (Decision D) is NOT added, fork PRs
would build `https://<deployment>/?x-vercel-protection-bypass=&x-vercel-set-bypass-cookie=true`,
get 401, and time out at `max_timeout` (120s after Decision B, but
still 4 minutes total wasted per fork PR across both workflows). The
fork-PR gate in Decision D prevents this entirely.

### Finding 5 — `max_timeout: 600` is excessive

PR #16's failed run (`gh run view 26370087240`) shows the wait-action
retrieved the deployment URL within 1 second (`target url » ...` at
T+1s relative to job start). The healthcheck then 401-looped for
~600s. With a working bypass, the first axios GET would have succeeded
within 2 seconds. **120s is plenty of headroom.** Decision B applies.

### Finding 6 — Concurrency expression is safe

Current `concurrency:` blocks:

- preview-smoke: `group: preview-smoke-${{ github.event.pull_request.number }}`
- visual-diff:   `group: visual-diff-${{ github.event.pull_request.number }}`

No secret reference. Adding the secret via the wait-action's `with:`
and the Playwright step's `env:` does NOT touch `concurrency:`. The
cancel-stale-runs behavior is preserved. Decision C applies.

### Finding 7 — `tests/smoke/app.smoke.spec.ts` is a `.ts` file in a JS-only repo

Out of scope for this brief — flagged for `adopt-playwright-smoke`,
which will own both the Playwright config and the JS/TS decision for
its test files. Do NOT rename or edit it here.

### Finding 8 — actionlint is not installed locally for the implementer

Not a blocker. The brief recommends installing it for local validation
(see Manual verification), but the absence of actionlint in CI today
means it's a recommended-not-required check. A future `adopt-actionlint`
convoy can add it to CI; this brief stays focused on the bypass plumb.

## Out of scope (do not do these)

- [ ] No new Playwright tests or `playwright.config.js`.
- [ ] No `@playwright/test` install.
- [ ] No `package.json` or `package-lock.json` changes.
- [ ] No edits to any other workflow YAML (`ci.yml`, etc.).
- [ ] No edits to `AGENTS.md` § 7 (the wording correction header → query param is the doc-writer pass after this convoy closes).
- [ ] No edits to `.cursor/rules/*.mdc`.
- [ ] No new vitest tests (the existing 16-test suite remains green and is unrelated to this convoy).
- [ ] No replacement of `patrickedqvist/wait-for-vercel-preview` with another action or a hand-rolled `gh api` + `curl` poll loop. That's `replace-wait-for-vercel-preview` (queued, separate scope) and is explicitly out of scope per the convoy file.
- [ ] No tightening or loosening of the `permissions:` blocks in either workflow — PR #16 landed the minimal scope.
- [ ] No `setup-node@v4` version bump, no `actions/checkout@v4` bump, no `actions/upload-artifact@v4` bump. Those are general dependency-bump scope, not this convoy's.
- [ ] No `max_timeout` change beyond the 600 → 120 specified by Decision B.
- [ ] No echo of the constructed URL or BASE_URL in any step. Even for "debugging." If a debug echo is needed during local iteration, remove it before committing.

## Rationale (≤3 sentences)

The wait-action's healthcheck loop is the de-facto bypass-works assertion; injecting the bypass via `path:` query is the only mechanism the action exposes (its `action.yml` has no custom-header input), and the action's source confirms the query never leaks to logs or `outputs.url`. Combining query-param-on-wait with header-form-on-Playwright (deferred to `adopt-playwright-smoke` via the plumb-the-env-var step) keeps both call sites idiomatic for their respective HTTP clients. The fork-PR gate and the `max_timeout` reduction are small operator-quality-of-life refinements that make a 10-minute failure into a 2-minute (or zero-minute) failure when something does go wrong.