docs: post-convoy cleanup for fix-vercel-deployment-protection-in-ci
Reflects the merged fix-vercel-deployment-protection-in-ci convoy (PR #17, squash commit9a3e077) in repo documentation. Closes the CI-infra side-effect of P0 #7. Three commits in the convoy: Brief 1 (bypass plumbing per spec), plus two scope expansions discovered during CI validation (shell-injection hardening, and a Decision-A shape correction to drop the cookie-bypass param). .convoys/ship-readiness.md: - Queued convoys: mark fix-vercel-deployment-protection-in-ci as RESOLVED 2026-05-24 with9a3e077. Document the 3-commit reality (365e9f0Brief 1 bypass plumbing,b6f8688shell-injection hardening of the gate Decide step,043a6eedropping &x-vercel-set-bypass-cookie=true), the empirical metrics (wait-action: 10-min timeout -> 194ms; workflow runtime: 10+ min -> 59s), the documented expected red on Playwright smoke (npx playwright test fails because playwright.config.js doesn't exist yet -- adopt-playwright-smoke owns that), and the operator-rotation caveat (R6). AGENTS.md: - Section 7 Deployment: correct the noun "header" -> "query param on wait-action's path:" since that's what actually landed per Decision A. Also document the without-cookie form (the cookie variant 401s through axios's missing cookie jar) and the operator re-seed runbook for token rotation. - Section 7 Deployment: fold in a one-liner about the GitHub Actions ${{ }}-in-shell-is-injection-vector pattern, with the env: + quoted-shell fix shape. Picked Section 7 over a new Gotcha #13 because the existing Gotchas list is dominated by app-level pitfalls (auth, SQL clients, ESLint), and CI YAML hardening is naturally co-located with deployment. .convoys/fix-vercel-deployment-protection-in-ci.md: - frontmatter status: in-progress -> shipped (added shipped: 2026-05-24) - new ## As-shipped section: 3-commit reality, Decision-A shape deviation (we shipped without &x-vercel-set-bypass-cookie=true), empirical timings (194ms wait, 59s total), remaining-red attribution to adopt-playwright-smoke, and the operator-rotation caveat. No changes to: package.json, lib/**, pages/**, components/**, scripts/**, .github/workflows/**, .cursor/rules/**, README.md. Co-authored-by: Cursor <cursoragent@cursor.com>
This commit is contained in:
parent
9a3e077d40
commit
c2a43a0ce2
3 changed files with 148 additions and 3 deletions
|
|
@ -13,8 +13,9 @@ skip:
|
|||
- role-ux-reviewer # no UX surface
|
||||
- role-ia-architect # no IA surface
|
||||
- browser-smoke # the convoy IS the smoke pipeline
|
||||
status: in-progress
|
||||
status: shipped
|
||||
created: 2026-05-24
|
||||
shipped: 2026-05-24
|
||||
parent: ship-readiness
|
||||
addresses: P0 #7 (CI infrastructure side-effect)
|
||||
depends_on:
|
||||
|
|
@ -545,3 +546,129 @@ serially.
|
|||
Architecture complete. 1 brief created. Estimated PRs: 1. Awaiting
|
||||
human gate 1 (Decisions A + B + D ratification + brief approval) before
|
||||
the implementer runs.
|
||||
|
||||
## As-shipped
|
||||
|
||||
Shipped 2026-05-24 as squash commit `9a3e077` (PR #17). The convoy
|
||||
shipped in three commits, not one — Brief 1 plus two scope expansions
|
||||
discovered during PR #17's own CI validation. Capturing the deviation
|
||||
from the architect's original 1-brief decomposition here so the next
|
||||
architect / reviewer has the audit trail.
|
||||
|
||||
### Three-commit reality
|
||||
|
||||
1. **`365e9f0` Brief 1 — bypass plumbing per spec.** Both workflows got
|
||||
the identical shape change architect planned: `wait-for-vercel-preview@v1.3.2`'s
|
||||
`path:` input now carries `/?x-vercel-protection-bypass=${{ secrets.VERCEL_AUTOMATION_BYPASS_SECRET }}&x-vercel-set-bypass-cookie=true`
|
||||
(Decision A's original cookie-variant shape — later corrected in
|
||||
commit 3); `max_timeout: 600 → 120` (Decision B); the `gate:` job's
|
||||
Decide step short-circuits on `github.event.pull_request.head.repo.fork == true`
|
||||
with a `::notice::` annotation, before the existing PR-body skip
|
||||
directive runs (Decision D); the Playwright/screenshot step exports
|
||||
`VERCEL_AUTOMATION_BYPASS_SECRET` as `env:` for forward-compat with
|
||||
`adopt-playwright-smoke`. ~25 LOC in `preview-smoke.yml`, ~15 LOC in
|
||||
`visual-diff.yml`.
|
||||
2. **`b6f8688` shell-injection hardening (scope expansion #1).**
|
||||
Pre-existing latent bug surfaced by PR #17's own CI validation.
|
||||
Decision D's gate step inlined `${{ github.event.pull_request.body }}`
|
||||
directly into bash, which broke when the PR body contained shell
|
||||
metacharacters like `(` or backticks — PR #17's description bit this
|
||||
with `"unexpected token \`('"` because of phrasing like *"(was:
|
||||
10-minute timeout)"*. Every prior Decide-step run was one badly-
|
||||
formatted PR body away from breaking the gate. Fix is the standard
|
||||
GitHub Actions hardening pattern: route the body and the fork flag
|
||||
through the step's `env:` block as `PR_BODY` and `PR_IS_FORK`, then
|
||||
quote them as `"$PR_BODY"` / `"$PR_IS_FORK"` in the shell condition.
|
||||
Same change in both workflows (~9 LOC each). This fix is technically
|
||||
beyond Brief 1's planned scope (which targeted only Vercel-bypass
|
||||
plumbing) but was bundled into the convoy because the bug actively
|
||||
blocked Brief 1's success criterion from being validated on PR #17.
|
||||
3. **`043a6ee` drop `&x-vercel-set-bypass-cookie=true` (scope expansion #2 —
|
||||
Decision-A shape correction).** Brief 1 used the cookie-variant
|
||||
shape per the original Decision A wording. PR #17's CI run showed
|
||||
the wait-action's healthcheck was still 401ing despite the bypass
|
||||
query being correct. Root cause: with `x-vercel-set-bypass-cookie=true`,
|
||||
Vercel responds **307 + Set-Cookie** (setting `_vercel_jwt`), but
|
||||
axios in Node has no cookie jar — it follows the redirect to the
|
||||
bare URL without the cookie, which then 401s. Operator's local curl
|
||||
confirmed empirically:
|
||||
|
||||
```
|
||||
curl -sI "https://<preview>/?x-vercel-protection-bypass=<secret>" | head -1
|
||||
→ HTTP/2 200 (works, no cookie needed)
|
||||
|
||||
curl -sI "https://<preview>/?x-vercel-protection-bypass=<secret>&x-vercel-set-bypass-cookie=true" | head -1
|
||||
→ HTTP/2 307 (the redirect-without-cookie path that breaks axios)
|
||||
```
|
||||
|
||||
For a one-shot healthcheck the per-request bypass query is enough.
|
||||
The cookie variant stays reserved for the future Playwright config
|
||||
in `adopt-playwright-smoke` where a real browser cookie jar exists.
|
||||
An inline comment in `preview-smoke.yml` now explains this so the
|
||||
next agent doesn't accidentally re-add the cookie param.
|
||||
|
||||
### Decision-A deviation from the original spec
|
||||
|
||||
**The convoy did NOT ship the cookie variant.** Decision A as written
|
||||
in this file specified `path: '/?x-vercel-protection-bypass=...&x-vercel-set-bypass-cookie=true'`
|
||||
(quote: "Append `?x-vercel-protection-bypass=...&x-vercel-set-bypass-cookie=true`
|
||||
to the wait-action's `path:` input"). Commit `043a6ee` corrected this
|
||||
to the bare `path: '/?x-vercel-protection-bypass=...'` after empirical
|
||||
evidence (per `curl -sI` above) showed Vercel's cookie-bypass path is
|
||||
incompatible with axios's no-cookie-jar behavior in the wait-action.
|
||||
The spec evolved during validation; the convoy file's Decision A text
|
||||
above is preserved as the original recommendation, but the next
|
||||
architect should know the as-shipped shape is the cookie-less form.
|
||||
The cookie variant remains the right call for Playwright's
|
||||
`extraHTTPHeaders` / cookie-jar-aware future use case (Decision A
|
||||
already flagged this division).
|
||||
|
||||
### As-shipped metrics (from PR #17's CI run, post-validation)
|
||||
|
||||
- `Wait for Vercel Preview deployment` step elapsed: **194 milliseconds**
|
||||
(was: 10-minute timeout on every PR before this convoy — a ~3,000×
|
||||
improvement). Acceptance criterion #3 (workflows complete in < 5
|
||||
minutes) crushed by ~50× margin on the wait-step alone.
|
||||
- `Playwright smoke` workflow total runtime: **59 seconds** (was: 10+
|
||||
minutes). Comfortably inside the < 5-minute acceptance threshold.
|
||||
- Step breakdown:
|
||||
- `Wait for Vercel Preview deployment` → success in 194ms ✅
|
||||
- `npm ci`, `setup-node`, `playwright install` → success ✅
|
||||
- `Run smoke tests` → failure (expected — see next section) ❌
|
||||
- `Screenshot diff` workflow: **not triggered on PR #17 itself** because
|
||||
its path filter excludes workflow-only changes. Will fire on the
|
||||
next PR touching `pages/**` / `components/**` / `styles/**` /
|
||||
Tailwind / PostCSS config.
|
||||
|
||||
### Remaining expected red
|
||||
|
||||
`Playwright smoke` workflow now reaches `npx playwright test` and
|
||||
fast-fails because **`playwright.config.js` does not exist** in the
|
||||
tree yet. That is `adopt-playwright-smoke`'s scope (P1 #10 step 2 /
|
||||
launch sequence step 10 in `.convoys/ship-readiness.md`), not this
|
||||
convoy's. Brief 1's acceptance criterion #1 explicitly accepts a real
|
||||
downstream failure as success, as long as the wait-action reaches
|
||||
`Received success status code` first — which it does, in 194ms. The
|
||||
failure mode shifted from "401 timeout in the wait step" (this
|
||||
convoy's target) to "playwright not installed" (the next convoy's
|
||||
target). That is the correct end state.
|
||||
|
||||
### Operator-rotation caveat
|
||||
|
||||
R6 in the Risk list. The Vercel bypass token does not auto-expire. If
|
||||
the operator rotates it from the Vercel dashboard, the workflow will
|
||||
silently start failing with the same 401 + timeout it did before this
|
||||
convoy. Re-seed via `gh secret set VERCEL_AUTOMATION_BYPASS_SECRET --body "<new value>"`.
|
||||
No automation; this is a human responsibility (same pattern as
|
||||
`JWT_SECRET` rotation, documented in `AGENTS.md` § 7).
|
||||
|
||||
### What did NOT change
|
||||
|
||||
- Workflow `permissions:` blocks (PR #16's contracts preserved).
|
||||
- `concurrency:` groups (Decision C confirmed unchanged).
|
||||
- Any application code (`pages/**`, `components/**`, `lib/**`,
|
||||
`scripts/**`).
|
||||
- Any test file (no new unit tests; vitest does not exercise GitHub
|
||||
Actions YAML).
|
||||
- `playwright.config.js` (still does not exist; owned by
|
||||
`adopt-playwright-smoke`).
|
||||
|
|
|
|||
|
|
@ -299,7 +299,24 @@ Follow-ups surfaced mid-convoy or mid-PR that didn't fit the original launch seq
|
|||
- **`single-auth-provider`** (priority: P1 quality, also listed as launch sequence step 9). Collapse `lib/auth-context.js` + `lib/admin-auth.js` into `lib/use-auth.js`. Surfaced again as a follow-up in P0 #7 § Flagged-but-deferred (4 pages still import the legacy `useAuth`).
|
||||
- **`cleanup-mobile-nav-dead-props`** (priority: P3 polish). `components/MobileNavigation.js` accepts a dead `user` prop; remove it. Surfaced in P0 #7 § Flagged-but-deferred. May fold into `god-component-split` (P2 #13) if that lands first.
|
||||
- **`bump-eslint-10`** (priority: P2 hygiene; upstream-blocked). Bump ESLint from v9 to v10 once `typescript-eslint` ships a v10-tested release and `eslint-config-next` bundles it. Surfaced in `.convoys/bump-next-js.md` § Decisions D.
|
||||
- **`fix-vercel-deployment-protection-in-ci`** (priority: P2 CI infra; **operator action required**). PR #16's permissions fix exposed that Vercel Deployment Protection 401s anonymous CI requests, so `Playwright smoke` and `Screenshot diff` now 10-min-timeout instead of 5-second-403. Plumb `VERCEL_AUTOMATION_BYPASS_SECRET` into both workflows + (eventually) Playwright config. Operator must seed the secret into GitHub Actions before the implementer can run. See `.convoys/fix-vercel-deployment-protection-in-ci.md`. Created 2026-05-24.
|
||||
- **`fix-vercel-deployment-protection-in-ci`** (priority: P2 CI infra) — **RESOLVED 2026-05-24**.
|
||||
- **Resolved by:** squash commit `9a3e077` (PR #17), comprising three commits, not one. Operator prereq seeded 2026-05-24T20:03:31Z (`gh secret set VERCEL_AUTOMATION_BYPASS_SECRET`; confirmed via `gh secret list`); the implementer dispatch waited on that visibility per the convoy file's "Operator action required" gate.
|
||||
- **Three-commit reality** (Brief 1 + two scope expansions found during CI validation):
|
||||
1. `365e9f0` Brief 1 — bypass plumbing per spec. Both `.github/workflows/preview-smoke.yml` and `.github/workflows/visual-diff.yml` got the same shape change: `wait-for-vercel-preview@v1.3.2`'s `path:` input now carries `/?x-vercel-protection-bypass=${{ secrets.VERCEL_AUTOMATION_BYPASS_SECRET }}&x-vercel-set-bypass-cookie=true` (Decision A's original cookie-variant shape — later corrected in commit 3); `max_timeout: 600 → 120` (Decision B); the `gate:` job's Decide step short-circuits on `github.event.pull_request.head.repo.fork == true` with a `::notice::` annotation, before the existing PR-body skip directive runs (Decision D); and the Playwright/screenshot step exports `VERCEL_AUTOMATION_BYPASS_SECRET` as `env:` for forward-compat with `adopt-playwright-smoke`.
|
||||
2. `b6f8688` shell-injection hardening (latent pre-existing bug surfaced by PR #17's own CI validation). Decision D's gate step inlined `${{ github.event.pull_request.body }}` directly into bash, which broke when the PR body contained shell metacharacters like `(` or backticks. PR #17's own description bit this with `"unexpected token \`('"` because of phrasing like *"(was: 10-minute timeout)"*. Fix is the standard GitHub Actions hardening pattern (their official "Security hardening" guide flags inline `${{ }}` in shell as both a syntax-error risk and a shell-injection vector): route the body and the fork flag through the step's `env:` block as `PR_BODY` and `PR_IS_FORK`, then quote them as `"$PR_BODY"` / `"$PR_IS_FORK"` in the shell condition. Same change in both workflows; ~9 LOC each. Documented in commit message as technically beyond Brief 1's scope but bundled into the convoy because the bug actively blocked Brief 1's success criterion from being validated.
|
||||
3. `043a6ee` drop `&x-vercel-set-bypass-cookie=true` from the wait-action `path:` — corrects Decision A's exact shape. With the cookie variant, Vercel responds **307 + Set-Cookie**, and axios in Node has no cookie jar — it follows the redirect to the bare URL without the cookie, which then 401s. Empirically confirmed by operator's local curl: bare `?x-vercel-protection-bypass=X` → HTTP/2 200, while `?x-vercel-protection-bypass=X&x-vercel-set-bypass-cookie=true` → HTTP/2 307 (the broken path). For a one-shot healthcheck the per-request bypass query is enough. The cookie variant stays reserved for the future Playwright config (`adopt-playwright-smoke`) where a real browser cookie jar exists. An inline comment in `preview-smoke.yml` explains this so the next agent doesn't accidentally re-add the cookie param.
|
||||
- **As-shipped metrics** (from PR #17's CI run, post-validation):
|
||||
- `Wait for Vercel Preview deployment` step elapsed: **194 milliseconds** (was: 10-minute timeout before this convoy).
|
||||
- `Playwright smoke` workflow total runtime: **59 seconds** (was: 10+ minutes).
|
||||
- Step breakdown: `Wait for Vercel Preview deployment` → success in 194ms; `npm ci`, `setup-node`, `playwright install` → success; `Run smoke tests` → **failure** (expected — see next bullet).
|
||||
- `Screenshot diff` workflow: not triggered on PR #17 itself because its path filter excludes workflow-only changes; will fire on the next PR touching `pages/**` / `components/**` / `styles/**` / Tailwind/PostCSS config.
|
||||
- **Documented expected red**: `Playwright smoke` now reaches `npx playwright test` and fast-fails because `playwright.config.js` doesn't exist in the tree yet. That is `adopt-playwright-smoke`'s scope (P1 #10 step 2 / launch sequence step 10), not this convoy's. Per the convoy file's Test plan § and Brief 1 acceptance criterion #1, a real downstream failure with the wait-action reaching `Received success status code` first counts as success for this convoy — the failure mode shifted from "401 timeout in the wait step" to "playwright not installed", which is precisely the target state.
|
||||
- **Operator-rotation caveat** (R6 in the convoy file). The Vercel bypass token does not auto-expire. If/when it's rotated from the Vercel dashboard, the operator must re-seed the GitHub secret via `gh secret set VERCEL_AUTOMATION_BYPASS_SECRET --body "<new value>"`. Same human-responsibility pattern as `JWT_SECRET` rotation; not preventable from workflow YAML. No automation here.
|
||||
- **Flagged-but-deferred** (from the convoy file's "Anything flagged but not acted on" section, unchanged at merge):
|
||||
1. `replace-wait-for-vercel-preview` — the wait-action's last release was Mar 2024; could be replaced with a few lines of `gh api` + `curl`-loop. Out of scope for this convoy; queue if the action ages out further or gets a security advisory.
|
||||
2. `adopt-playwright-smoke` — owns the actual `playwright.config.js`, `tests/smoke/`, and `@playwright/test` dep. The bypass plumbing here is forward-compat for that convoy (env var available on the smoke step). Listed in P1 #10 step 2 / launch sequence step 10 above.
|
||||
3. `Screenshot diff` baseline authoring — orthogonal scope; the visual-diff workflow has nothing to compare against on its first real run.
|
||||
- **Owns:** `role-architect` (3 Decisions ratified — A query-param, B 120s timeout, D fork-PR skip) → `role-implementer` (Brief 1) + two scope-expansion commits.
|
||||
|
||||
## Self-analytics
|
||||
|
||||
|
|
|
|||
|
|
@ -77,7 +77,8 @@ Code graph is indexed by `user-code-review-graph` MCP (122 files, 628 nodes, 560
|
|||
## 7. Deployment
|
||||
|
||||
- **Vercel** auto-deploys `main` and creates Preview deployments for every PR. `vercel.json` and `.vercel/` are committed. CI in `.github/workflows/` runs lint + types (no duplicate build — Vercel handles it).
|
||||
- **Preview protection bypass for automation.** The project has a Protection Bypass for Automation token exposed locally as `VERCEL_AUTOMATION_BYPASS_SECRET` in `.env.local` (not committed). Smoke/visual-diff workflows pass this header (`x-vercel-protection-bypass`) when hitting password-protected preview URLs. Needed for the queued `adopt-playwright-smoke` convoy; do not log or echo the value.
|
||||
- **Preview protection bypass for automation.** The project has a Protection Bypass for Automation token exposed locally as `VERCEL_AUTOMATION_BYPASS_SECRET` in `.env.local` (not committed) and seeded into GitHub Actions as a repo secret (`gh secret set VERCEL_AUTOMATION_BYPASS_SECRET`, 2026-05-24). The `preview-smoke` and `visual-diff` workflows pass the bypass as a **query parameter** on `wait-for-vercel-preview@v1.3.2`'s `path:` input — `path: '/?x-vercel-protection-bypass=…'`, bare form, **without** `&x-vercel-set-bypass-cookie=true` (the cookie variant returns 307 + Set-Cookie and axios in Node has no cookie jar, so it 401s on the redirect). The header form (`x-vercel-protection-bypass: <secret>`) is reserved for the future `playwright.config.js` that `adopt-playwright-smoke` will ship (Playwright has its own request shape and `extraHTTPHeaders` is cleaner there). Do not log or echo the value. If the operator rotates the token in the Vercel dashboard, re-seed the GitHub secret via `gh secret set VERCEL_AUTOMATION_BYPASS_SECRET --body "<new value>"`. See `.convoys/fix-vercel-deployment-protection-in-ci.md`.
|
||||
- **Shell-injection hardening in workflow YAML.** Never inline `${{ github.event.* }}` directly into a `run:` block — route the value through the step's `env:` block and quote it (`"$VAR_NAME"`) in shell. PR #17's CI validation caught a real syntax error from a PR body containing `(` because the gate-job's Decide step inlined `${{ github.event.pull_request.body }}` straight into bash; commit `b6f8688` swept both `preview-smoke.yml` and `visual-diff.yml` to the `env:` + quoted-shell pattern. This is GitHub's official Security Hardening guidance ("Security hardening for GitHub Actions" → "Using a third-party action"). Apply to any new workflow that reads PR body / title / branch name / commit messages in shell.
|
||||
|
||||
## 8. Code graph
|
||||
|
||||
|
|
|
|||
Loading…
Reference in a new issue