Skip to content

Commit e50a349

Browse files
authored
fix(cypress): move CSP frame-ancestors from Next.js middleware to ingress layer (#5025)
1 parent 558e27d commit e50a349

13 files changed

Lines changed: 116 additions & 63 deletions

File tree

.factory/skills/klicker-cypress-e2e/SKILL.md

Lines changed: 65 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -65,6 +65,71 @@ gh run download <run-id> --repo uzh-bf/klicker-uzh --dir /tmp/klicker-cypress-ru
6565

6666
After you identify the failing spec path in logs, reproduce locally with `test:run:one`.
6767

68+
## CI mass-failure triage workflow (Cypress Cloud + GH)
69+
70+
Use this when failures explode across many specs (for example, fail count rising continuously in Cypress Cloud).
71+
72+
### 1) Monitor run completion first
73+
74+
```bash
75+
# quick status of the two key checks on a PR
76+
gh pr checks <pr-number> --json name,bucket,state,description,link \
77+
--jq '.[] | select(.name=="cypress-run-cloud" or .name=="cypress: default-group (merge)") | [.name,.bucket,.state,(.description//"-"),.link] | @tsv'
78+
79+
# inspect cypress workflow job/step state
80+
gh run view <run-id> --json jobs \
81+
--jq '.jobs[] | select(.name=="cypress-run-cloud") | {status,conclusion,steps:[.steps[]|{n:.number,name:.name,status:.status,conclusion:.conclusion}]}'
82+
```
83+
84+
Important: `gh run view --log-failed` and `--job ... --log` only work after the run/job has completed.
85+
86+
### 2) Capture logs immediately after completion
87+
88+
```bash
89+
gh run view <run-id> --log-failed > /tmp/cypress-run-<run-id>-failed.log
90+
gh run view <run-id> --job <job-id> --log > /tmp/cypress-run-<run-id>-job-<job-id>.log
91+
```
92+
93+
### 3) Classify failure signatures
94+
95+
```bash
96+
rg -n "Failed to load static props|unhandled promise rejection|Y\\.prefetch|_next/static/chunks/main" \
97+
/tmp/cypress-run-<run-id>-failed.log /tmp/cypress-run-<run-id>-job-<job-id>.log
98+
```
99+
100+
Buckets:
101+
102+
- prefetch/static-props crash signatures (`Failed to load static props`, `Y.prefetch`, Next chunk stack)
103+
- broad app-level unhandled promise rejection signatures
104+
- ordinary selector/timeout/assertion failures
105+
106+
### 4) Decision tree
107+
108+
- If prefetch/static-props signatures appear across many specs:
109+
- treat as middleware interception regression
110+
- harden middleware in all three apps:
111+
- `apps/frontend-manage/src/middleware.ts`
112+
- `apps/frontend-control/src/middleware.ts`
113+
- `apps/frontend-pwa/src/middleware.ts`
114+
- skip CSP mutation on prefetch requests (`next-router-prefetch`, `purpose=prefetch`, equivalent headers)
115+
- broaden matcher exclusion from selective `_next/static|_next/image|_next/data` to all `_next`
116+
- keep runtime `ALLOWED_FRAME_ANCESTORS` behavior (do not move back to build-time-only CSP config)
117+
- If signatures are absent and failures are localized:
118+
- split by spec and fix as isolated test/app issues instead of a global middleware rollback
119+
120+
### 5) Post-fix validation gate
121+
122+
```bash
123+
# optional fast local smoke on critical specs
124+
pnpm --filter @klicker-uzh/cypress test:run:one --spec cypress/e2e/0-baseline-ops.cy.ts
125+
pnpm --filter @klicker-uzh/cypress test:run:one --spec cypress/e2e/A-login-workflow.cy.ts
126+
127+
# CI must not show the catastrophic signature anymore
128+
gh run view <new-run-id> --log-failed | rg -n "Failed to load static props|unhandled promise rejection|Y\\.prefetch"
129+
```
130+
131+
Passing this gate means Cypress can still have isolated flakes, but no suite-wide early-collapse pattern.
132+
68133
## Cypress debugging cookbook
69134

70135
### 1) Reduce the problem

AGENTS.md

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -205,6 +205,8 @@ Without Traefik, use `http://localhost:<port>` directly. The `*.klicker.com` dom
205205
- **PR review triage**: Copilot/CodeRabbit/SonarCloud flag many false positives. Always check if guards/fallbacks already exist before "fixing" reported issues. Confirm with the actual code, not the bot summary.
206206
- **agent-browser via npx**: Always use `npx agent-browser` instead of bare `agent-browser`. Global install conflicts with Volta's Node shim and fails with "Could not execute command".
207207
- **LTI launch target resolver contract**: Launch targets are resolved in strict precedence `custom claim (klicker_redirect_to)` -> `query redirectTo`; no env fallback is used in resolver logic. Validation fails closed on the first present invalid source and enforces URL hostname exact/subdomain checks against `COOKIE_DOMAIN` and `DF_DOMAIN` (never substring matching). (`apps/lti/src/launchTarget.ts`)
208+
- **CSP frame-ancestors via ingress, not middleware**: Pages Router apps (manage, pwa, control) must NOT use Next.js middleware for CSP -- it breaks `_next/data` routes in production builds (known Next.js bug). CSP `frame-ancestors` is set at the reverse proxy layer: HAProxy ingress annotations in K8s (`haproxy.org/response-set-header`), Traefik `customResponseHeaders` middleware in local dev. (`deploy/charts/klicker-uzh-v3/templates/ingress-*.yaml`, `util/traefik/rules_docker.yaml`)
209+
- **Cypress CI signal timing**: `cypress: default-group (merge)` can report an increasing failed-test count while `cypress-run-cloud` is still in progress; wait for `cypress-run-cloud` completion before expecting downloadable GitHub job logs. (`.github/workflows/cypress-testing.yml`)
208210
- **Infisical + Turbo env sync**: Any Infisical-managed env var used by tasks must be listed in `turbo.json` `globalEnv`; otherwise task runs/cache behavior can become stale or inconsistent across environments.
209211
- **Participant email uniqueness across auth modes**: Prisma enforces `Participant @@unique([email, isSSOAccount])`, so the same normalized email can exist once as manual and once as SSO. To block new cross-mode duplicates, account creation must explicitly check normalized email collisions in service logic. (`packages/graphql/src/services/accounts.ts`)
210212
- **Helm v3 secrets are external**: `deploy/charts/klicker-uzh-v3/` deployments reference `envFrom.secretRef` names, but the chart currently defines no `Secret` manifests; secrets must be provisioned out-of-band with matching names. (`deploy/charts/klicker-uzh-v3/templates/`)

apps/frontend-control/src/middleware.ts

Lines changed: 0 additions & 17 deletions
This file was deleted.

apps/frontend-manage/src/middleware.ts

Lines changed: 0 additions & 17 deletions
This file was deleted.

apps/frontend-pwa/src/middleware.ts

Lines changed: 0 additions & 17 deletions
This file was deleted.

deploy/charts/klicker-uzh-v3/templates/cm-frontend-control.yaml

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,3 @@ metadata:
88
data:
99
# SSR-only GraphQL endpoint for Control frontend (internal service DNS)
1010
API_URL_SSR: "http://{{ include "chart.fullname" . }}-backend-graphql:3000/api/graphql"
11-
{{- if .Values.frontendControl.allowedFrameAncestors }}
12-
ALLOWED_FRAME_ANCESTORS: {{ .Values.frontendControl.allowedFrameAncestors | quote }}
13-
{{- end }}

deploy/charts/klicker-uzh-v3/templates/cm-frontend-manage.yaml

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,3 @@ metadata:
88
data:
99
# SSR-only GraphQL endpoint for Manage frontend (internal service DNS)
1010
API_URL_SSR: "http://{{ include "chart.fullname" . }}-backend-graphql:3000/api/graphql"
11-
{{- if .Values.frontendManage.allowedFrameAncestors }}
12-
ALLOWED_FRAME_ANCESTORS: {{ .Values.frontendManage.allowedFrameAncestors | quote }}
13-
{{- end }}

deploy/charts/klicker-uzh-v3/templates/cm-frontend-pwa.yaml

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,3 @@ metadata:
88
data:
99
# SSR-only GraphQL endpoint for PWA (internal service DNS)
1010
API_URL_SSR: "http://{{ include "chart.fullname" . }}-backend-graphql:3000/api/graphql"
11-
{{- if .Values.frontendPWA.allowedFrameAncestors }}
12-
ALLOWED_FRAME_ANCESTORS: {{ .Values.frontendPWA.allowedFrameAncestors | quote }}
13-
{{- end }}

deploy/charts/klicker-uzh-v3/templates/ingress-chat.yaml

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -12,10 +12,16 @@ metadata:
1212
name: {{ $fullName }}-chat
1313
labels:
1414
{{- include "chart.ingressLabels" . | nindent 4 }}
15-
{{- with .Values.chat.ingress.annotations }}
15+
{{- if or .Values.chat.ingress.annotations .Values.chat.allowedFrameAncestors }}
1616
annotations:
17+
{{- with .Values.chat.ingress.annotations }}
1718
{{- toYaml . | nindent 4 }}
1819
{{- end }}
20+
{{- if .Values.chat.allowedFrameAncestors }}
21+
haproxy.org/response-set-header: |
22+
Content-Security-Policy "frame-ancestors 'self' {{ .Values.chat.allowedFrameAncestors }}"
23+
{{- end }}
24+
{{- end }}
1925
spec:
2026
ingressClassName: {{ .Values.chat.ingress.className }}
2127
{{- if .Values.chat.ingress.tls }}

deploy/charts/klicker-uzh-v3/templates/ingress-frontend-control.yaml

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -12,10 +12,16 @@ metadata:
1212
name: {{ $fullName }}-frontend-control
1313
labels:
1414
{{- include "chart.ingressLabels" . | nindent 4 }}
15-
{{- with .Values.frontendControl.ingress.annotations }}
15+
{{- if or .Values.frontendControl.ingress.annotations .Values.frontendControl.allowedFrameAncestors }}
1616
annotations:
17+
{{- with .Values.frontendControl.ingress.annotations }}
1718
{{- toYaml . | nindent 4 }}
1819
{{- end }}
20+
{{- if .Values.frontendControl.allowedFrameAncestors }}
21+
haproxy.org/response-set-header: |
22+
Content-Security-Policy "frame-ancestors 'self' {{ .Values.frontendControl.allowedFrameAncestors }}"
23+
{{- end }}
24+
{{- end }}
1925
spec:
2026
ingressClassName: {{ .Values.frontendControl.ingress.className }}
2127
{{- if .Values.frontendControl.ingress.tls }}

0 commit comments

Comments
 (0)