enhance: add issuer and scope to signJWT, app origins for everything#4903
Conversation
📝 WalkthroughWalkthroughCentralizes app origin configuration via APP_ORIGIN_* across env files, CI, and Helm. Introduces issuer support in JWT utilities and updates all signers/callers. Adds SSR GraphQL endpoint env (API_URL_SSR) to frontends and Helm ConfigMaps. Adds startup env validations. Adjusts Prisma scripts wrapper and an Office add-in build script. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant Caller as Service (Auth/PWA/LTI/GraphQL)
participant Util as util.signJWT
participant Jose as jose Sign/Verify
participant Consumer as Token Consumer
Caller->>Util: signJWT(payload, secret, { issuer, issuedAt, expiresIn })
Util->>Jose: build JWT (setIssuer if provided)
Jose-->>Caller: JWT
Caller->>Consumer: Use JWT (header/cookie/param)
Consumer->>Jose: verifyJWT(token, key, { issuer })
Jose-->>Consumer: Payload (or error)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested labels
Suggested reviewers
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 Pre-merge checks❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
|
️✅ There are no secrets present in this pull request anymore.If these secrets were true positive and are still valid, we highly recommend you to revoke them. 🦉 GitGuardian detects secrets in your source code to help developers and security teams secure the modern development process. You are seeing this because you or someone else with access to this repository has authorized GitGuardian to scan your pull request. |
dosubot
Bot
added
the
size:L
…into jwt-issuer-v2 # Conflicts: # packages/graphql/src/services/liveQuizzes.ts # packages/util/src/jwt.ts
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 8
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (4)
cypress/cypress/support/commands.ts (1)
142-149: Cypress env vs process.env: issuer will be undefined in browser context
process.env.JWT_ISSUER_AUTHis unreliable in Cypress test (browser) context. UseCypress.env('JWT_ISSUER_AUTH')and fail fast if missing. Pair this with passingCYPRESS_JWT_ISSUER_AUTHin the workflow.Apply:
cy.wrap(null).then(async () => { - const token = await new jose.SignJWT(tokenData) + const issuer = Cypress.env('JWT_ISSUER_AUTH') as string | undefined + if (!issuer) { + throw new Error('JWT_ISSUER_AUTH is not defined for Cypress tests') + } + const token = await new jose.SignJWT(tokenData) .setProtectedHeader({ alg }) .setIssuedAt() .setExpirationTime('2h') - .setIssuer(process.env.JWT_ISSUER_AUTH) + .setIssuer(issuer) .sign(secret)packages/graphql/src/services/accounts.ts (1)
236-248: Do not leak magic-link tokens in notifications.Teams message below (Lines 264-267) includes the full JWT. That’s a security risk if channels/logs are exposed. Send metadata only or redact.
await sendTeamsNotification({ scope: 'graphql/sendMagicLink', - text: `One-time login token created for ${usernameOrEmail}: ${magicLink}`, + text: `One-time login link created for ${usernameOrEmail} (valid 15m)`, })apps/frontend-pwa/src/lib/getParticipantToken.ts (1)
133-138: Cookie maxAge uses milliseconds; nookies expects seconds.Current value sets ~35 years instead of 13 days. Use seconds.
- nookies.set(ctx, 'participant_token', participantToken, { + nookies.set(ctx, 'participant_token', participantToken, { domain: process.env.COOKIE_DOMAIN, path: '/', httpOnly: true, - maxAge: 1000 * 60 * 60 * 24 * 13, + maxAge: 60 * 60 * 24 * 13, // 13 days in seconds secure:apps/lti/src/index.ts (1)
152-154: NaN port bug: defaulting with ?? doesn’t catch NaN.Number(undefined) yields NaN, which is not null/undefined, so 4000 isn’t used. Parse with a default string.
- const result = await Provider.deploy({ - port: Number(process.env.LTI_PORT) ?? 4000, - }) + const result = await Provider.deploy({ + port: parseInt(process.env.LTI_PORT ?? '4000', 10), + })
🧹 Nitpick comments (12)
apps/backend-docker/src/index.ts (1)
81-88: Type the invalidate payload instead of any.Prevents accidental shape drift and enables autocomplete.
-emitter.on('invalidate', (resource: any) => { +type InvalidateResource = { typename: string; id: string | number } +emitter.on('invalidate', (resource: InvalidateResource) => {packages/util/src/jwt.ts (1)
22-27: Export option types to de-duplicate across callers.Multiple files now mirror these shapes. Exporting typed aliases reduces drift.
-export async function signJWT( +export type SignJWTOptions = { + algorithm?: 'HS256' + expiresIn?: string | number + issuer?: string + issuedAt?: Date +} + +export async function signJWT( payload: JWTPayload, secret: string, - options: { - algorithm?: 'HS256' - expiresIn?: string | number - issuer?: string - issuedAt?: Date - } = {} + options: SignJWTOptions = {} ): Promise<string> { ... } -export async function verifyJWT( +export type VerifyJWTOptions = { + algorithms?: 'HS256'[] + clockTolerance?: string | number + issuer?: string +} + +export async function verifyJWT( token: string, secret: string, - opts: { - algorithms?: 'HS256'[] - clockTolerance?: string | number - issuer?: string - } = {} + opts: VerifyJWTOptions = {} ): Promise<JWTPayload> { ... }Also applies to: 48-52
packages/graphql/src/scripts/impersonateParticipant.ts (1)
26-28: Guard against missing issuer to keep tokens consistent.Fail early if JWT_ISSUER_API is unset; otherwise the script silently signs without iss.
async function run(username: string) { + if (!process.env.JWT_ISSUER_API) { + console.error('JWT_ISSUER_API must be set') + process.exit(1) + }packages/graphql/src/scripts/impersonateUser.ts (1)
29-31: Same guard for issuer as in participant script.Avoid producing tokens without iss in local runs.
async function run(email: string) { + if (!process.env.JWT_ISSUER_API) { + console.error('JWT_ISSUER_API must be set') + process.exit(1) + }packages/graphql/src/services/liveQuizzes.ts (2)
1427-1431: Include a narrow JWT scope and avoid 1970 iat fallback.Add a purpose‑specific scope to prevent token confusion and use a current timestamp fallback instead of epoch to avoid validators rejecting ancient iat.
- { - issuer: process.env.JWT_ISSUER_API, - issuedAt: updatedQuiz.activeBlock!.startedAt ?? new Date(0), - } + { + issuer: process.env.JWT_ISSUER_API, + issuedAt: updatedQuiz.activeBlock!.startedAt ?? new Date(), + }And in the payload above, add a scope:
{ instanceId: instance.id, execution: updatedQuiz.activeBlock!.execution, liveQuizId: quiz.id, sub: '', // dummy sub, since this value is required + scope: 'lq:correlation-key', },
2959-2963: Mirror the scope + iat fallback adjustments here, too.Keep correlationKey claims consistent across both call sites.
- { - issuer: process.env.JWT_ISSUER_API, - issuedAt: quiz.activeBlock?.startedAt ?? new Date(0), - } + { + issuer: process.env.JWT_ISSUER_API, + issuedAt: quiz.activeBlock?.startedAt ?? new Date(), + }And extend the payload above:
{ instanceId: instance.id, execution: quiz.activeBlock!.execution, liveQuizId: quiz.id, sub: '', // dummy sub, since this value is required + scope: 'lq:correlation-key', },deploy/charts/klicker-uzh-v2/templates/cm-frontend-pwa.yaml (1)
16-16: PWA issuer added to ConfigMap.Good. This key is unconditional, so PWA pods will always get it; ensure values are non-empty in all envs.
deploy/charts/klicker-uzh-v2/templates/cm-backend-graphql.yaml (1)
52-52: Fail fast in Helm when issuer is unset.Guard the value with required() to avoid deploying an instance that will crash at runtime.
Apply:
- JWT_ISSUER_API: {{ .Values.jwtIssuers.api | quote }} + JWT_ISSUER_API: {{ required "values.jwtIssuers.api is required" .Values.jwtIssuers.api | quote }}Please confirm the backend Deployment mounts this ConfigMap key into env so apps/backend-docker/src/index.ts sees JWT_ISSUER_API at startup.
apps/frontend-pwa/src/pages/createAccount.tsx (1)
120-128: Optionally verify issuer for LTI1.3, if available.To tighten validation without breaking flows, pass issuer when defined.
- const parsedToken = (await verifyJWT( - token, - process.env.APP_SECRET as string - )) as { + const parsedToken = (await verifyJWT( + token, + process.env.APP_SECRET as string, + process.env.JWT_ISSUER_LTI ? { issuer: process.env.JWT_ISSUER_LTI } : undefined + )) as {apps/frontend-pwa/src/lib/getParticipantToken.ts (1)
104-117: Reduce duplication with createAccount.tsx.The LTI1.1 signing block is duplicated. Extract a small helper (e.g., signLtiJwtLti11(payload)) to centralize claims/options and reduce drift.
apps/lti/src/index.ts (2)
66-72: Remove redundant runtime env check.This duplicates the module-load guard; keep one source of truth.
- if (!process.env.JWT_ISSUER_LTI) { - console.error( - 'JWT_ISSUER_LTI environment variable is required but not defined' - ) - process.exit(1) - }
33-38: Cast DB port to number.Sequelize expects a number; ensure type correctness.
- port: process.env.LTI_DB_PORT ?? 5432, + port: parseInt(process.env.LTI_DB_PORT ?? '5432', 10),
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (29)
.github/workflows/cypress-testing.yml(2 hunks).github/workflows/test-graphql.yml(1 hunks)apps/auth/src/pages/api/auth/[...nextauth].ts(3 hunks)apps/backend-docker/src/index.ts(2 hunks)apps/docs/package.json(1 hunks)apps/frontend-pwa/.env.assessment(1 hunks)apps/frontend-pwa/.env.assessment.qa(1 hunks)apps/frontend-pwa/.env.development(1 hunks)apps/frontend-pwa/.env.production(1 hunks)apps/frontend-pwa/.env.qa(1 hunks)apps/frontend-pwa/.env.test(1 hunks)apps/frontend-pwa/src/lib/getParticipantToken.ts(3 hunks)apps/frontend-pwa/src/pages/createAccount.tsx(2 hunks)apps/lti/src/index.ts(3 hunks)cypress/cypress/support/commands.ts(1 hunks)deploy/charts/klicker-uzh-v2/templates/cm-auth.yaml(1 hunks)deploy/charts/klicker-uzh-v2/templates/cm-backend-assessment.yaml(1 hunks)deploy/charts/klicker-uzh-v2/templates/cm-backend-graphql.yaml(1 hunks)deploy/charts/klicker-uzh-v2/templates/cm-frontend-assessment.yaml(1 hunks)deploy/charts/klicker-uzh-v2/templates/cm-frontend-pwa.yaml(1 hunks)deploy/charts/klicker-uzh-v2/templates/cm-lti.yaml(1 hunks)deploy/charts/klicker-uzh-v2/values.yaml(1 hunks)deploy/env-prod-v3/values.yaml(1 hunks)deploy/env-qa-v3/values.yaml(1 hunks)packages/graphql/src/scripts/impersonateParticipant.ts(1 hunks)packages/graphql/src/scripts/impersonateUser.ts(1 hunks)packages/graphql/src/services/accounts.ts(4 hunks)packages/graphql/src/services/liveQuizzes.ts(2 hunks)packages/util/src/jwt.ts(2 hunks)
🧰 Additional context used
🧠 Learnings (4)
📚 Learning: 2025-06-23T12:32:13.205Z
Learnt from: CR
PR: uzh-bf/klicker-uzh#0
File: apps/backend-docker/CLAUDE.md:0-0
Timestamp: 2025-06-23T12:32:13.205Z
Learning: Authentication is implemented using Passport.js with JWT strategy, extracting tokens from specific cookies or the Authorization header, and verifying them with the APP_SECRET environment variable.
Applied to files:
packages/graphql/src/scripts/impersonateParticipant.tspackages/graphql/src/scripts/impersonateUser.tsapps/auth/src/pages/api/auth/[...nextauth].ts
📚 Learning: 2025-09-08T19:16:04.791Z
Learnt from: sjschlapbach
PR: uzh-bf/klicker-uzh#4884
File: apps/frontend-pwa/src/components/liveQuiz/LiveQuizSubscriber.tsx:21-37
Timestamp: 2025-09-08T19:16:04.791Z
Learning: For RunningLiveQuizUpdatedDocument subscription in apps/frontend-pwa/src/components/liveQuiz/LiveQuizSubscriber.tsx, the backend implementation guarantees non-null values, so frontend null checks would be unnecessary and falsify return types. The subscription schema should be updated to reflect non-nullable return types instead.
Applied to files:
packages/graphql/src/services/liveQuizzes.ts
📚 Learning: 2025-06-23T12:33:32.937Z
Learnt from: CR
PR: uzh-bf/klicker-uzh#0
File: packages/next-config/CLAUDE.md:0-0
Timestamp: 2025-06-23T12:33:32.937Z
Learning: When backend models (e.g., Prisma schema) change, update any dependent frontend types to maintain type safety and consistency.
Applied to files:
apps/backend-docker/src/index.ts
📚 Learning: 2025-06-23T12:32:13.205Z
Learnt from: CR
PR: uzh-bf/klicker-uzh#0
File: apps/backend-docker/CLAUDE.md:0-0
Timestamp: 2025-06-23T12:32:13.205Z
Learning: The GraphQL server uses GraphQL Yoga with plugins for security (GraphQL Armor), CSRF prevention, persisted operations, and optional response caching.
Applied to files:
apps/backend-docker/src/index.ts
🧬 Code graph analysis (1)
apps/auth/src/pages/api/auth/[...nextauth].ts (1)
packages/util/src/jwt.ts (2)
signJWT(19-43)JWTPayload(3-12)
🔇 Additional comments (19)
apps/frontend-pwa/.env.qa (1)
19-19: Add JWT_ISSUER_PWA — looks goodValue matches NEXT_PUBLIC_PWA_URL; consistent with Helm wiring. No further changes needed.
apps/frontend-pwa/.env.assessment.qa (1)
19-19: Assessment issuer added — OKIssuer matches assessment PWA URL. Keep this in sync with Helm values (assessmentPwa).
apps/frontend-pwa/.env.production (1)
21-21: Prod issuer added — OKMatches public PWA domain; aligns with production values.yaml.
deploy/env-prod-v3/values.yaml (1)
4-10: Introduce jwtIssuers block — looks consistentAll prod issuers are HTTPS and match public hostnames used elsewhere. Ensure templates default to empty string or fail-fast if any value is omitted.
If not already present, consider a simple values schema/validation in CI to assert these keys exist.
apps/frontend-pwa/.env.development (1)
19-19: Dev issuer added — OKMatches dev PWA URL. Keep consistent with other env files; no trailing slash (good).
.github/workflows/test-graphql.yml (1)
129-129: Expose JWT_ISSUER_API to tests — confirm tests read itprocess.env.JWT_ISSUER_API is used when signing/verifying tokens in: packages/graphql/src/services/liveQuizzes.ts (lines 1428, 2960); packages/graphql/src/services/accounts.ts (lines 49, 65, 246, 705); packages/graphql/src/scripts/impersonateParticipant.ts (line 27); packages/graphql/src/scripts/impersonateUser.ts (line 30). No direct matches in test files were found — ensure .github/workflows/test-graphql.yml exports JWT_ISSUER_API and that tests or test helpers read process.env.JWT_ISSUER_API when creating/verifying tokens.
packages/util/src/jwt.ts (1)
25-27: Issuer wiring in sign/verify looks correct.Issuer is optional in both paths; default behavior is preserved.
Also applies to: 38-41, 57-57
apps/auth/src/pages/api/auth/[...nextauth].ts (2)
629-631: Scope tagging for participant tokens looks fine.Setting token.scope = 'EDUID' on initial sign-in is consistent with issuer-stamping.
142-145: Confirm JWT expiry behavior for NextAuth tokensencode() signs the JWT with only issuer — no exp is set here. Confirm exp is added elsewhere and that it aligns with session.maxAge / NEXTAUTH_JWT_MAX_AGE; if not, set expiresIn when signing.
File: apps/auth/src/pages/api/auth/[...nextauth].ts (lines 142–145)
- return signJWT((token as JWTPayload) ?? {}, secretString, { - issuer: process.env.JWT_ISSUER_AUTH, - }) + return signJWT((token as JWTPayload) ?? {}, secretString, { + issuer: process.env.JWT_ISSUER_AUTH, + expiresIn: process.env.NEXTAUTH_JWT_MAX_AGE ?? '30d', + })Quick local check (decode a real token and confirm "exp"):
node -e 'console.log(JSON.parse(Buffer.from(process.argv[2].split(".")[1],"base64").toString()))' ""
deploy/env-qa-v3/values.yaml (1)
4-10: Typo breaks assessmentApi issuer URL (leading space).This introduces an invalid URL and will break issuer-based verification/signing.
lti: 'https://lti.klicker-qa.bf-app.ch' pwa: 'https://pwa.klicker-qa.bf-app.ch' - assessmentApi: 'https:// assessment-api.klicker-qa.bf-app.ch' + assessmentApi: 'https://assessment-api.klicker-qa.bf-app.ch' assessmentPwa: 'https://assessment.klicker-qa.bf-app.ch'Likely an incorrect or invalid review comment.
packages/graphql/src/services/accounts.ts (1)
49-50: Gate strict JWT issuer verification behind a feature flagCreation now stamps iss; verification still accepts any issuer — add a feature flag (e.g., JWT_ENFORCE_ISSUER === 'true') to enable strict issuer checking, update all verifyJWT call sites across the repo, then flip the flag once rollout confirms all tokens include iss.
- const tokenData = (await verifyJWT( - token, - process.env.APP_SECRET as string - )) as { + const tokenData = (await verifyJWT( + token, + process.env.APP_SECRET as string, + process.env.JWT_ENFORCE_ISSUER === 'true' + ? { issuer: process.env.JWT_ISSUER_API } + : {} + )) as {Find call sites: rg -nP '\bverifyJWT\s*(' -C2
apps/frontend-pwa/.env.test (1)
20-20: LGTM: issuer for PWA added to test env.Matches local URLs; aligns with issuer-backed signing in PWA flows.
deploy/charts/klicker-uzh-v2/templates/cm-frontend-assessment.yaml (1)
9-9: PWA issuer for assessment added.Consistent with other ConfigMaps; no issues spotted.
apps/frontend-pwa/.env.assessment (1)
19-19: LGTM: issuer matches public assessment PWA URL.Aligns with NEXT_PUBLIC_PWA_URL; consistent issuer string.
deploy/charts/klicker-uzh-v2/templates/cm-backend-assessment.yaml (1)
48-48: Assessment API issuer added — verify env coverage and backend usagedeploy/charts/klicker-uzh-v2/templates/cm-backend-assessment.yaml:48 sets JWT_ISSUER_API: {{ .Values.jwtIssuers.assessmentApi | quote }}; search returned no references to JWT_ISSUER_API in packages/, util/ or apps/. Ensure each env values file defines jwtIssuers.assessmentApi and that the backend reads JWT_ISSUER_API for signing/verifying.
apps/frontend-pwa/src/pages/createAccount.tsx (1)
146-151: Good: issuer embedded and guarded.Runtime check for JWT_ISSUER_PWA and passing issuer to signJWT look correct and align with the new contract.
Also applies to: 162-163
apps/frontend-pwa/src/lib/getParticipantToken.ts (1)
98-103: Good: guarded LTI1.1 path + issuer in token.The runtime check and issuer option are correct.
Also applies to: 115-116
apps/lti/src/index.ts (1)
6-13: Good: fail-fast env validation at startup.Early exit on missing JWT_ISSUER_LTI is appropriate for a standalone service.
deploy/charts/klicker-uzh-v2/values.yaml (1)
39-46: LGTM: central issuer config block.The jwtIssuers map is clear and aligns with the templates.
Please ensure env-prod-v3/values.yaml and env-qa-v3/values.yaml populate all six issuers with non-empty values to match the new runtime guards.
| JWT_ISSUER_AUTH: http://127.0.0.1:3010 | ||
| JWT_ISSUER_API: http://127.0.0.1:3000 |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Prefix for Cypress env mapping
Expose these to the Cypress browser via CYPRESS_ prefix so Cypress.env() can read them.
Apply:
- JWT_ISSUER_AUTH: http://127.0.0.1:3010
- JWT_ISSUER_API: http://127.0.0.1:3000
+ CYPRESS_JWT_ISSUER_AUTH: http://127.0.0.1:3010
+ CYPRESS_JWT_ISSUER_API: http://127.0.0.1:3000📝 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.
| JWT_ISSUER_AUTH: http://127.0.0.1:3010 | |
| JWT_ISSUER_API: http://127.0.0.1:3000 | |
| CYPRESS_JWT_ISSUER_AUTH: http://127.0.0.1:3010 | |
| CYPRESS_JWT_ISSUER_API: http://127.0.0.1:3000 |
🤖 Prompt for AI Agents
.github/workflows/cypress-testing.yml around lines 196 to 197: the workflow
exposes JWT_ISSUER_AUTH and JWT_ISSUER_API to the job but Cypress.browser can
only read env vars prefixed with CYPRESS_; rename or duplicate these environment
keys to include the CYPRESS_ prefix (e.g., set CYPRESS_JWT_ISSUER_AUTH:
http://127.0.0.1:3010 and CYPRESS_JWT_ISSUER_API: http://127.0.0.1:3000) so
tests can access them via Cypress.env().
| JWT_ISSUER_AUTH: http://127.0.0.1:3010 | ||
| JWT_ISSUER_API: http://127.0.0.1:3000 |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Same here: use CYPRESS_ prefix
Mirror the change for the Cloud-recording job.
Apply:
- JWT_ISSUER_AUTH: http://127.0.0.1:3010
- JWT_ISSUER_API: http://127.0.0.1:3000
+ CYPRESS_JWT_ISSUER_AUTH: http://127.0.0.1:3010
+ CYPRESS_JWT_ISSUER_API: http://127.0.0.1:3000📝 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.
| JWT_ISSUER_AUTH: http://127.0.0.1:3010 | |
| JWT_ISSUER_API: http://127.0.0.1:3000 | |
| CYPRESS_JWT_ISSUER_AUTH: http://127.0.0.1:3010 | |
| CYPRESS_JWT_ISSUER_API: http://127.0.0.1:3000 |
🤖 Prompt for AI Agents
.github/workflows/cypress-testing.yml around lines 382 to 383: environment
variables JWT_ISSUER_AUTH and JWT_ISSUER_API are using the wrong prefix for
Cypress jobs; rename them to use the CYPRESS_ prefix (e.g.,
CYPRESS_JWT_ISSUER_AUTH and CYPRESS_JWT_ISSUER_API) in the Cloud-recording job
to mirror the other job changes so Cypress picks up the values as test env vars.
…sment apps get the same env var so verification should be consistent)
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (6)
deploy/charts/klicker-uzh-v2/templates/cm-response-api.yaml (1)
14-17: Optional: render only when setTo avoid injecting empty envs in local/dev, gate rendering on non-empty values.
- JWT_ISSUER_API: {{ .Values.jwtIssuers.api | quote }} + {{- if .Values.jwtIssuers.api }} + JWT_ISSUER_API: {{ .Values.jwtIssuers.api | quote }} + {{- end }} - JWT_ISSUER_AUTH: {{ .Values.jwtIssuers.auth | quote }} + {{- if .Values.jwtIssuers.auth }} + JWT_ISSUER_AUTH: {{ .Values.jwtIssuers.auth | quote }} + {{- end }} - JWT_ISSUER_LTI: {{ .Values.jwtIssuers.lti | quote }} + {{- if .Values.jwtIssuers.lti }} + JWT_ISSUER_LTI: {{ .Values.jwtIssuers.lti | quote }} + {{- end }} - JWT_ISSUER_PWA: {{ .Values.jwtIssuers.pwa | quote }} + {{- if .Values.jwtIssuers.pwa }} + JWT_ISSUER_PWA: {{ .Values.jwtIssuers.pwa | quote }} + {{- end }}deploy/charts/klicker-uzh-v2/templates/cm-response-api-assessment.yaml (1)
14-17: Optional: conditional rendering like other templatesSame gating suggestion as for response-api to avoid empty entries.
- JWT_ISSUER_API: {{ .Values.jwtIssuers.assessmentApi | quote }} + {{- if .Values.jwtIssuers.assessmentApi }} + JWT_ISSUER_API: {{ .Values.jwtIssuers.assessmentApi | quote }} + {{- end }} - JWT_ISSUER_PWA: {{ .Values.jwtIssuers.assessmentPwa | quote }} + {{- if .Values.jwtIssuers.assessmentPwa }} + JWT_ISSUER_PWA: {{ .Values.jwtIssuers.assessmentPwa | quote }} + {{- end }}deploy/charts/klicker-uzh-v2/templates/cm-frontend-assessment.yaml (1)
9-12: Optional: omit empty valuesMirror the conditional rendering pattern to keep local dev flexible.
- JWT_ISSUER_AUTH: {{ .Values.jwtIssuers.auth | quote }} + {{- if .Values.jwtIssuers.auth }} + JWT_ISSUER_AUTH: {{ .Values.jwtIssuers.auth | quote }} + {{- end }}deploy/charts/klicker-uzh-v2/templates/cm-backend-graphql.yaml (1)
52-55: Minor: consider centralized include for issuer blockThese four lines repeat across many templates; extract to a named template to reduce drift.
+# _jwt-issuers.tpl (new helper) +{{- define "klicker.jwtIssuers.standard" -}} +{{- if .Values.jwtIssuers.api }}JWT_ISSUER_API: {{ .Values.jwtIssuers.api | quote }}{{ end }} +{{- if .Values.jwtIssuers.auth }}\nJWT_ISSUER_AUTH: {{ .Values.jwtIssuers.auth | quote }}{{ end }} +{{- if .Values.jwtIssuers.lti }}\nJWT_ISSUER_LTI: {{ .Values.jwtIssuers.lti | quote }}{{ end }} +{{- if .Values.jwtIssuers.pwa }}\nJWT_ISSUER_PWA: {{ .Values.jwtIssuers.pwa | quote }}{{ end }} +{{- end }}Then in this file:
- JWT_ISSUER_API: {{ .Values.jwtIssuers.api | quote }} - JWT_ISSUER_AUTH: {{ .Values.jwtIssuers.auth | quote }} - JWT_ISSUER_LTI: {{ .Values.jwtIssuers.lti | quote }} - JWT_ISSUER_PWA: {{ .Values.jwtIssuers.pwa | quote }} + {{- include "klicker.jwtIssuers.standard" . | nindent 2 }}deploy/charts/klicker-uzh-v2/templates/cm-hatchet-workers.yaml (2)
13-18: Deduplicate issuer blocks across both worker ConfigMapsBoth sections repeat six keys; extract a helper to avoid drift and keep one source of truth.
+# _jwt-issuers.tpl (extend helper) +{{- define "klicker.jwtIssuers.standard" -}} +{{- if .Values.jwtIssuers.api }}JWT_ISSUER_API: {{ .Values.jwtIssuers.api | quote }}{{ end }} +{{- if .Values.jwtIssuers.auth }}\nJWT_ISSUER_AUTH: {{ .Values.jwtIssuers.auth | quote }}{{ end }} +{{- if .Values.jwtIssuers.lti }}\nJWT_ISSUER_LTI: {{ .Values.jwtIssuers.lti | quote }}{{ end }} +{{- if .Values.jwtIssuers.pwa }}\nJWT_ISSUER_PWA: {{ .Values.jwtIssuers.pwa | quote }}{{ end }} +{{- end }} +{{- define "klicker.jwtIssuers.assessment" -}} +{{- if .Values.jwtIssuers.assessmentApi }}JWT_ISSUER_ASSESSMENT_API: {{ .Values.jwtIssuers.assessmentApi | quote }}{{ end }} +{{- if .Values.jwtIssuers.assessmentPwa }}\nJWT_ISSUER_ASSESSMENT_PWA: {{ .Values.jwtIssuers.assessmentPwa | quote }}{{ end }} +{{- end }}Then replace both repeated blocks with:
- JWT_ISSUER_API: {{ .Values.jwtIssuers.api | quote }} - JWT_ISSUER_AUTH: {{ .Values.jwtIssuers.auth | quote }} - JWT_ISSUER_LTI: {{ .Values.jwtIssuers.lti | quote }} - JWT_ISSUER_PWA: {{ .Values.jwtIssuers.pwa | quote }} - JWT_ISSUER_ASSESSMENT_API: {{ .Values.jwtIssuers.assessmentApi | quote }} - JWT_ISSUER_ASSESSMENT_PWA: {{ .Values.jwtIssuers.assessmentPwa | quote }} + {{- include "klicker.jwtIssuers.standard" . | nindent 2 }} + {{- include "klicker.jwtIssuers.assessment" . | nindent 2 }}Also applies to: 31-36
13-18: Optional: gate on non-empty valuesIf dev installs keep defaults as empty strings, consider conditional rendering to avoid empty envs.
- JWT_ISSUER_ASSESSMENT_API: {{ .Values.jwtIssuers.assessmentApi | quote }} + {{- if .Values.jwtIssuers.assessmentApi }} + JWT_ISSUER_ASSESSMENT_API: {{ .Values.jwtIssuers.assessmentApi | quote }} + {{- end }}Also applies to: 31-36
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (9)
deploy/charts/klicker-uzh-v2/templates/cm-auth.yaml(1 hunks)deploy/charts/klicker-uzh-v2/templates/cm-backend-assessment.yaml(1 hunks)deploy/charts/klicker-uzh-v2/templates/cm-backend-graphql.yaml(1 hunks)deploy/charts/klicker-uzh-v2/templates/cm-frontend-assessment.yaml(1 hunks)deploy/charts/klicker-uzh-v2/templates/cm-frontend-pwa.yaml(1 hunks)deploy/charts/klicker-uzh-v2/templates/cm-hatchet-workers.yaml(2 hunks)deploy/charts/klicker-uzh-v2/templates/cm-lti.yaml(1 hunks)deploy/charts/klicker-uzh-v2/templates/cm-response-api-assessment.yaml(1 hunks)deploy/charts/klicker-uzh-v2/templates/cm-response-api.yaml(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (4)
- deploy/charts/klicker-uzh-v2/templates/cm-lti.yaml
- deploy/charts/klicker-uzh-v2/templates/cm-auth.yaml
- deploy/charts/klicker-uzh-v2/templates/cm-frontend-pwa.yaml
- deploy/charts/klicker-uzh-v2/templates/cm-backend-assessment.yaml
🔇 Additional comments (5)
deploy/charts/klicker-uzh-v2/templates/cm-response-api-assessment.yaml (1)
14-17: LGTM: assessment issuers mapped correctlyAPI→assessmentApi and PWA→assessmentPwa are consistent; shared AUTH/LTI reused. All six keys (api, auth, lti, pwa, assessmentApi, assessmentPwa) exist in deploy/charts/klicker-uzh-v2/values.yaml (lines 40–45); auth also appears at line 101 and lti at line 301.
deploy/charts/klicker-uzh-v2/templates/cm-hatchet-workers.yaml (1)
13-18: Verify worker env consumption matches Helm valuesSearch returned no matches; verification inconclusive. Confirm both worker processes read these exact env vars or update the chart: JWT_ISSUER_API, JWT_ISSUER_AUTH, JWT_ISSUER_LTI, JWT_ISSUER_PWA, JWT_ISSUER_ASSESSMENT_API, JWT_ISSUER_ASSESSMENT_PWA — deploy/charts/klicker-uzh-v2/templates/cm-hatchet-workers.yaml (lines 13–18, 31–36).
deploy/charts/klicker-uzh-v2/templates/cm-response-api.yaml (1)
14-17: LGTM — issuer envs wired for response-api; verify runtime usageNames and quoting match the chart values. ripgrep returned "No files were searched" for the prior check. Re-run this in the repo to confirm the env names are referenced at runtime:
#!/bin/bash rg -nC2 -S --hidden --no-ignore -g '!**/node_modules/**' -P '\bJWT_ISSUER_(API|AUTH|LTI|PWA)\b'deploy/charts/klicker-uzh-v2/templates/cm-backend-graphql.yaml (1)
52-55: LGTM: GraphQL backend wired with base issuersConsistent naming; quoting OK. Found assessment templates at deploy/charts/klicker-uzh-v2/templates (cm-backend-assessment.yaml, secret-backend-assessment.yaml) — confirm the assessment backend declares its own JWT issuers in the assessment secret to avoid mixing.
deploy/charts/klicker-uzh-v2/templates/cm-frontend-assessment.yaml (1)
9-12: LGTM: frontend-assessment picks assessment issuersIssuer keys and sources align with the assessment context. Automated repo search returned no matches — confirm the frontend actually consumes these env names (process.env.JWT_ISSUER_{API,AUTH,LTI,PWA}, import.meta.env.VITE_JWT_ISSUER_, or REACT_APP_JWT_ISSUER_). Run from repo root:
rg -nC2 -S -uu "process\.env\.(JWT_ISSUER_(API|AUTH|LTI|PWA)|REACT_APP_JWT_ISSUER_(API|AUTH|LTI|PWA)|VITE_JWT_ISSUER_(API|AUTH|LTI|PWA))|import\.meta\.env\.(VITE_JWT_ISSUER_(API|AUTH|LTI|PWA)|JWT_ISSUER_(API|AUTH|LTI|PWA))" apps || true
…into jwt-issuer-v2 # Conflicts: # deploy/charts/klicker-uzh-v2/templates/cm-backend-assessment.yaml # deploy/charts/klicker-uzh-v2/templates/cm-frontend-pwa.yaml # deploy/charts/klicker-uzh-v2/templates/cm-hatchet-workers.yaml # deploy/charts/klicker-uzh-v2/templates/cm-response-api-assessment.yaml
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
apps/backend-docker/.env.cypress (1)
14-16: Appease dotenv-linter: drop quotes (optional) and/or reorder keys.Quotes are harmless for most loaders but trigger dotenv-linter warnings. If you want a clean lint run, remove quotes; reordering is purely stylistic.
Apply to remove quotes:
-JWT_ISSUER_AUTH="http://127.0.0.1:3010" -JWT_ISSUER_API="http://127.0.0.1:3000" -JWT_ISSUER_PWA="http://127.0.0.1:3002" +JWT_ISSUER_AUTH=http://127.0.0.1:3010 +JWT_ISSUER_API=http://127.0.0.1:3000 +JWT_ISSUER_PWA=http://127.0.0.1:3002If you also want alphabetical ordering, place the JWT_ISSUER_* block above REDIS_CACHE_HOST.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
.github/workflows/cypress-testing.yml(2 hunks)apps/backend-docker/.env.cypress(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- .github/workflows/cypress-testing.yml
🧰 Additional context used
🪛 dotenv-linter (3.3.0)
apps/backend-docker/.env.cypress
[warning] 14-14: [QuoteCharacter] The value has quote characters (', ")
(QuoteCharacter)
[warning] 14-14: [UnorderedKey] The JWT_ISSUER_AUTH key should go before the REDIS_CACHE_HOST key
(UnorderedKey)
[warning] 15-15: [QuoteCharacter] The value has quote characters (', ")
(QuoteCharacter)
[warning] 15-15: [UnorderedKey] The JWT_ISSUER_API key should go before the JWT_ISSUER_AUTH key
(UnorderedKey)
[warning] 16-16: [QuoteCharacter] The value has quote characters (', ")
(QuoteCharacter)
[warning] 16-16: [UnorderedKey] The JWT_ISSUER_PWA key should go before the REDIS_CACHE_HOST key
(UnorderedKey)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (11)
- GitHub Check: Analyze (javascript)
- GitHub Check: build
- GitHub Check: build
- GitHub Check: build
- GitHub Check: build
- GitHub Check: format
- GitHub Check: cypress-run-cloud
- GitHub Check: SonarCloud
- GitHub Check: check
- GitHub Check: test
- GitHub Check: test
🔇 Additional comments (1)
apps/backend-docker/.env.cypress (1)
14-16: Confirm issuer endpoints/ports used by Cypress — verification requiredRepo search returned no matches; apps/backend-docker/.env.cypress (lines 14–16) contains:
JWT_ISSUER_AUTH="http://127.0.0.1:3010"
JWT_ISSUER_API="http://127.0.0.1:3000"
JWT_ISSUER_PWA="http://127.0.0.1:3002"Verify these map to the actual local services Cypress runs against (port mismatches will break issuer verification).
…into jwt-issuer-v2 # Conflicts: # deploy/charts/klicker-uzh-v2/templates/cm-hatchet-workers.yaml # deploy/charts/klicker-uzh-v2/templates/cm-response-api.yaml
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (16)
deploy/charts/klicker-uzh-v2/templates/cm-backend-assessment.yaml (1)
46-49: Prefer safe defaults and avoid empty-string envs for assessment issuersUse Helm’s default to fall back to non-assessment issuers when assessment-specific values are unset, and avoid injecting empty strings.
- JWT_ISSUER_API: {{ .Values.jwtIssuers.assessmentApi | quote }} + JWT_ISSUER_API: {{ default .Values.jwtIssuers.api .Values.jwtIssuers.assessmentApi | quote }} - JWT_ISSUER_AUTH: {{ .Values.jwtIssuers.auth | quote }} + JWT_ISSUER_AUTH: {{ .Values.jwtIssuers.auth | quote }} - JWT_ISSUER_LTI: {{ .Values.jwtIssuers.lti | quote }} + JWT_ISSUER_LTI: {{ .Values.jwtIssuers.lti | quote }} - JWT_ISSUER_PWA: {{ .Values.jwtIssuers.assessmentPwa | quote }} + JWT_ISSUER_PWA: {{ default .Values.jwtIssuers.pwa .Values.jwtIssuers.assessmentPwa | quote }}Also verify all services expect the exact same issuer string format (scheme, host, trailing slash) to prevent verification mismatches.
deploy/charts/klicker-uzh-v2/templates/cm-hatchet-workers.yaml (3)
21-26: Clarify necessity of JWT_ISSUER_ASSESSMENT_ in “general” workers*The general workers now expose both base and assessment issuer vars. If no code consumes JWT_ISSUER_ASSESSMENT_API/PWA here, drop them to reduce config drift; otherwise, document usage. Consider default fallbacks to avoid empty strings.
- JWT_ISSUER_ASSESSMENT_API: {{ .Values.jwtIssuers.assessmentApi | quote }} - JWT_ISSUER_ASSESSMENT_PWA: {{ .Values.jwtIssuers.assessmentPwa | quote }} + # Uncomment if explicitly consumed by workers; keep stable defaults if set. + # JWT_ISSUER_ASSESSMENT_API: {{ default .Values.jwtIssuers.api .Values.jwtIssuers.assessmentApi | quote }} + # JWT_ISSUER_ASSESSMENT_PWA: {{ default .Values.jwtIssuers.pwa .Values.jwtIssuers.assessmentPwa | quote }}
47-50: Add defensive defaults for non-assessment workersMirror the defaulting pattern so workers don’t receive empty-string issuers when values are omitted.
- JWT_ISSUER_API: {{ .Values.jwtIssuers.api | quote }} + JWT_ISSUER_API: {{ .Values.jwtIssuers.api | quote }} - JWT_ISSUER_AUTH: {{ .Values.jwtIssuers.auth | quote }} + JWT_ISSUER_AUTH: {{ .Values.jwtIssuers.auth | quote }} - JWT_ISSUER_LTI: {{ .Values.jwtIssuers.lti | quote }} + JWT_ISSUER_LTI: {{ .Values.jwtIssuers.lti | quote }} - JWT_ISSUER_PWA: {{ .Values.jwtIssuers.pwa | quote }} + JWT_ISSUER_PWA: {{ .Values.jwtIssuers.pwa | quote }}If any of these can be intentionally empty, consider gating with
with/ifto omit the key instead of setting"".
72-75: Align assessment workers with safe defaults and issuer normalizationUse defaults to base values when assessment-specific issuers are unset, and confirm all components use identical issuer string formatting to pass strict verification.
- JWT_ISSUER_API: {{ .Values.jwtIssuers.assessmentApi | quote }} + JWT_ISSUER_API: {{ default .Values.jwtIssuers.api .Values.jwtIssuers.assessmentApi | quote }} - JWT_ISSUER_AUTH: {{ .Values.jwtIssuers.auth | quote }} + JWT_ISSUER_AUTH: {{ .Values.jwtIssuers.auth | quote }} - JWT_ISSUER_LTI: {{ .Values.jwtIssuers.lti | quote }} + JWT_ISSUER_LTI: {{ .Values.jwtIssuers.lti | quote }} - JWT_ISSUER_PWA: {{ .Values.jwtIssuers.assessmentPwa | quote }} + JWT_ISSUER_PWA: {{ default .Values.jwtIssuers.pwa .Values.jwtIssuers.assessmentPwa | quote }}Side note: adjacency shows Redis cache values here still reference non-assessment
.Values.backendGraphql.redisCache.*; confirm that’s intentional for assessment workers.packages/graphql/test/helpers.ts (1)
842-851: Harden PIN generation for assessment LQ (test determinism/usability)Minor: consider excluding ambiguous chars and making the test PIN deterministic to avoid flaky assertions.
- pinCode: course?.isAssessmentEnabled - ? generatePassword.generate({ + pinCode: course?.isAssessmentEnabled + ? generatePassword.generate({ length: 6, numbers: true, uppercase: true, lowercase: false, symbols: false, + excludeSimilarCharacters: true, }) : null,If tests assert on the value, prefer a seeded/mocked generator.
apps/frontend-pwa/src/components/CourseElement.tsx (2)
72-79: Tailwind class typo: 'rounded-l-none!' should be '!rounded-l-none'Trailing exclamation is invalid; Tailwind’s important modifier is a leading '!'.
- 'rounded-l-none! h-full p-3', + '!rounded-l-none h-full p-3',
76-83: Cursor style ignores overall disabled stateWhen disabled is true (tile disabled) but pushDisabled is false, the button still shows a pointer even though onClick early-returns.
- !course.isSubscribed && !pushDisabled && 'cursor-pointer' + !course.isSubscribed && !pushDisabled && !disabled && 'cursor-pointer'apps/frontend-manage/src/components/activities/actions/useLiveQuizActions.ts (1)
251-268: Update useMemo deps to include setResetModal (and optionally quiz.courseId)setResetModal is referenced in the memoized actions but missing from deps; add it to satisfy exhaustive-deps and prevent stale closures.
Apply:
setDeletionModal, setActivityLogOpen, + setResetModal, + // Optional: used inside unpublish closure + // quiz.courseId,packages/i18n/messages/de.ts (1)
2281-2285: Fix German singular/plural and hyphenation in reset confirmation copy.Use singular “Assessment‑Live‑Quiz” and add missing article “alle” for points.
- resetLiveQuizMessage: - 'Bitte bestätigen Sie das Zurücksetzen dieses Assessment-Live Quizzes. Alle Antworten der Studierenden und gesammelten Punkte werden gelöscht. Diese Aktion wird im Audit-Log dokumentiert und kann nicht rückgängig gemacht werden.', + resetLiveQuizMessage: + 'Bitte bestätigen Sie das Zurücksetzen dieses Assessment-Live-Quiz. Alle Antworten der Studierenden und alle gesammelten Punkte werden gelöscht. Diese Aktion wird im Audit-Log dokumentiert und kann nicht rückgängig gemacht werden.',packages/graphql/test/assessmentRestrictions.test.ts (4)
321-346: Avoid hardcoding ElementInstance IDs in tests.Specifying a fixed
id: 49384risks collisions with auto‑incrementing IDs across runs. Let Prisma assign the ID.elements: { create: [ { - id: 49384, type: ElementInstanceType.LIVE_QUIZ, elementType: ElementType.SC, order: 0, elementData: processElementData(SC), options: {}, results: getInitialInstanceResults(processElementData(SC)), anonymousResults: getInitialInstanceResults( processElementData(SC) ), elementId: SC.id, ownerId: userOneCtx.user.sub, }, ], },
371-395: Prefer typed variable overascast for response payload.Keeps the shape aligned with
SingleQuestionResponseChoicesand avoids unnecessary casting.- await prisma.liveQuizResponse.create({ - data: { + const response: SingleQuestionResponseChoices = { + choices: [ + { ix: 0, selected: true }, + { ix: 1, selected: false }, + { ix: 2, selected: true }, + ], + } + await prisma.liveQuizResponse.create({ + data: { submittedAt: new Date(), - response: { - choices: [ - { ix: 0, selected: true }, - { ix: 1, selected: false }, - { ix: 2, selected: true }, - ], - } as SingleQuestionResponseChoices, + response, timeSpent: 1, correctness: 'CORRECT',
330-339: Micro‑nit: avoid repeated processing of element data.Compute
const scData = processElementData(SC)once and reuse forresultsandanonymousResults.
464-470: Test doesn’t actually validate owner can reset an ENDED quiz.You call owner after admin already reset to DRAFT, so it returns null for the wrong reason. Add a separate ENDED quiz and assert owner can reset it.
- // owner can reset as well; but quiz is already reset to DRAFT, so calling again should return null (no longer ENDED) - const res8 = await resetAssessmentLiveQuiz( - { id: endedQuiz.id }, - userOneCtx - ) - expect(res8).toBeNull() + // owner can reset an ENDED quiz as well + const endedQuizOwner = await seedLiveQuiz( + { + elements: [{ id: SC.id, type: ElementType.SC }], + status: PublicationStatus.ENDED, + courseId: assessment.id, + }, + userOneCtx + ) + const res8 = await resetAssessmentLiveQuiz( + { id: endedQuizOwner.id }, + userOneCtx + ) + expect(res8).not.toBeNull() + expect(res8?.status).toEqual(PublicationStatus.DRAFT)packages/graphql/src/services/liveQuizzes.ts (2)
2534-2535: Guard against negative shared-users countIf no direct permissions exist,
_count.permissions - 1can be negative. Clamp to zero.Apply:
- numSharedUsers: updatedQuiz._count.permissions - 1, + numSharedUsers: Math.max(0, updatedQuiz._count.permissions - 1),
2377-2395: Service-level permission filter duplicates resolver checksPrior guidance: permission checks live in resolvers (
checkAccess), services focus on data ops. Consider removing thecourse.permissions.somefilter here and relying on resolver checks to avoid drift and double maintenance.Context: See learning from 2025-04-17 about resolver-level checks for live-quiz ops.
apps/frontend-manage/src/components/courses/modals/LiveQuizResetModal.tsx (1)
85-88: Handle mutation errors and null response to avoid silent failureWrap
resetLiveQuiz()in try/catch and bail when the mutation returnsnull(e.g., permission or state mismatch).Apply:
- onSubmit={async () => { - await resetLiveQuiz() - await onSuccess?.() - }} + onSubmit={async () => { + try { + const { data } = await resetLiveQuiz() + if (!data?.resetAssessmentLiveQuiz) return + await onSuccess?.() + } catch (_) { + // TODO: surface an error toast/snackbar + } + }}
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (6)
packages/graphql/src/graphql/ops/MResetAssessmentLiveQuiz.graphqlis excluded by!**/**/graphql/ops/**packages/graphql/src/ops.schema.jsonis excluded by!**/**/ops.schema.jsonpackages/graphql/src/ops.tsis excluded by!**/**/ops.tspackages/graphql/src/public/client.jsonis excluded by!**/**/public/**packages/graphql/src/public/schema.graphqlis excluded by!**/**/public/**packages/graphql/src/public/server.jsonis excluded by!**/**/public/**
📒 Files selected for processing (19)
.github/workflows/test-graphql.yml(2 hunks)apps/frontend-manage/src/components/activities/actions/useLiveQuizActions.ts(4 hunks)apps/frontend-manage/src/components/activities/overview/LiveQuizActions.tsx(6 hunks)apps/frontend-manage/src/components/courses/modals/LiveQuizResetModal.tsx(1 hunks)apps/frontend-pwa/src/components/CourseElement.tsx(2 hunks)deploy/charts/klicker-uzh-v2/templates/cm-backend-assessment.yaml(1 hunks)deploy/charts/klicker-uzh-v2/templates/cm-backend-graphql.yaml(1 hunks)deploy/charts/klicker-uzh-v2/templates/cm-frontend-pwa.yaml(1 hunks)deploy/charts/klicker-uzh-v2/templates/cm-hatchet-workers.yaml(3 hunks)deploy/charts/klicker-uzh-v2/templates/cm-response-api-assessment.yaml(1 hunks)deploy/charts/klicker-uzh-v2/templates/cm-response-api.yaml(1 hunks)packages/graphql/src/schema/mutation.ts(1 hunks)packages/graphql/src/schema/query.ts(3 hunks)packages/graphql/src/services/liveQuizzes.ts(4 hunks)packages/graphql/test/assessmentRestrictions.test.ts(2 hunks)packages/graphql/test/helpers.ts(2 hunks)packages/i18n/messages/de.ts(2 hunks)packages/i18n/messages/en.ts(4 hunks)packages/shared-components/src/hooks/usePushNotifications.ts(1 hunks)
✅ Files skipped from review due to trivial changes (2)
- packages/shared-components/src/hooks/usePushNotifications.ts
- packages/graphql/src/schema/query.ts
🚧 Files skipped from review as they are similar to previous changes (5)
- deploy/charts/klicker-uzh-v2/templates/cm-response-api.yaml
- deploy/charts/klicker-uzh-v2/templates/cm-backend-graphql.yaml
- deploy/charts/klicker-uzh-v2/templates/cm-response-api-assessment.yaml
- deploy/charts/klicker-uzh-v2/templates/cm-frontend-pwa.yaml
- .github/workflows/test-graphql.yml
🧰 Additional context used
🧠 Learnings (3)
📚 Learning: 2025-04-17T12:52:17.002Z
Learnt from: sjschlapbach
PR: uzh-bf/klicker-uzh#4606
File: packages/graphql/src/services/liveQuizzes.ts:1100-1106
Timestamp: 2025-04-17T12:52:17.002Z
Learning: In the application architecture, permission checks for operations like activateLiveQuizBlock, deactivateLiveQuizBlock, endLiveQuiz, cancelLiveQuiz, and deleteLiveQuiz are performed at the GraphQL mutation resolver level using the checkAccess function, rather than in the service functions themselves. This separation of concerns means that ownerId filters within service functions are no longer necessary.
Applied to files:
packages/graphql/src/schema/mutation.tspackages/graphql/test/assessmentRestrictions.test.tspackages/graphql/src/services/liveQuizzes.ts
📚 Learning: 2025-04-17T12:47:09.182Z
Learnt from: sjschlapbach
PR: uzh-bf/klicker-uzh#4625
File: packages/graphql/src/services/liveQuizzes.ts:771-779
Timestamp: 2025-04-17T12:47:09.182Z
Learning: In the KlickerUZH system, running live quizzes (with status PublicationStatus.PUBLISHED) cannot be deleted as prevented by validation logic in the deleteLiveQuiz function, making the isDeleted: false filter redundant when querying for published quizzes.
Applied to files:
apps/frontend-manage/src/components/activities/overview/LiveQuizActions.tsx
📚 Learning: 2025-09-08T19:16:04.791Z
Learnt from: sjschlapbach
PR: uzh-bf/klicker-uzh#4884
File: apps/frontend-pwa/src/components/liveQuiz/LiveQuizSubscriber.tsx:21-37
Timestamp: 2025-09-08T19:16:04.791Z
Learning: For RunningLiveQuizUpdatedDocument subscription in apps/frontend-pwa/src/components/liveQuiz/LiveQuizSubscriber.tsx, the backend implementation guarantees non-null values, so frontend null checks would be unnecessary and falsify return types. The subscription schema should be updated to reflect non-nullable return types instead.
Applied to files:
packages/graphql/src/services/liveQuizzes.ts
🧬 Code graph analysis (3)
packages/graphql/src/schema/mutation.ts (2)
packages/graphql/src/schema/activities.ts (1)
ActivityInfo(128-199)packages/graphql/src/services/sharing.ts (1)
withPermission(5896-5918)
packages/graphql/test/assessmentRestrictions.test.ts (4)
packages/graphql/test/helpers.ts (3)
seedAnswerCollections(382-409)seedElements(418-546)seedLiveQuiz(813-876)packages/graphql/src/schema/elementData.ts (2)
ElementType(25-27)ElementInstanceType(33-35)packages/util/src/elements.ts (2)
processElementData(53-183)getInitialInstanceResults(185-297)packages/graphql/src/services/liveQuizzes.ts (1)
resetAssessmentLiveQuiz(2373-2553)
packages/graphql/src/services/liveQuizzes.ts (3)
packages/graphql/src/lib/context.ts (1)
ContextWithUser(37-46)packages/util/src/elements.ts (1)
getInitialInstanceResults(185-297)packages/graphql/src/services/activities.ts (1)
getPermissionBooleans(55-86)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (12)
- GitHub Check: build
- GitHub Check: cypress-run-cloud
- GitHub Check: test
- GitHub Check: build
- GitHub Check: Analyze (javascript)
- GitHub Check: build
- GitHub Check: build
- GitHub Check: check
- GitHub Check: test
- GitHub Check: build
- GitHub Check: SonarCloud
- GitHub Check: format
🔇 Additional comments (12)
apps/frontend-pwa/src/components/CourseElement.tsx (1)
46-48: LGTM: container/link sizing adjustmentsUsing items-stretch and h-full flex-1 on LinkButton improves alignment when the subscribe action is present.
packages/i18n/messages/en.ts (1)
2247-2251: LGTM: adds reset strings and fixes “lose” typoNew reset-related messages read well and the typo fix is correct.
Also applies to: 2266-2269, 2344-2346, 2615-2616
apps/frontend-manage/src/components/activities/overview/LiveQuizActions.tsx (1)
16-16: Reset action wiring looks correct; confirm permission semantics and modal flow
- Status and permission gating for resetLiveQuiz is sensible (Ended + assessment + reviewer).
- Modal integration and refetch on success look good.
Please verify:
- useLiveQuizActions exposes a resetLiveQuiz action that calls setResetModal(true).
- isActivityReviewer matches “assessment course admin” as intended for this capability.
- LiveQuizResetModal closes itself on success (or call setResetModal(false) in onSuccess if not).
Also applies to: 67-75, 105-105, 172-176, 216-217, 265-275
packages/graphql/src/schema/mutation.ts (1)
1421-1432: Reset mutation is correctly gated and consistent with our access modelUsing t.withAuth(asUserFullAccess) + withPermission(..., ADMIN) matches deleteLiveQuiz and similar admin-only ops. Return type ActivityInfo also aligns with create/edit flows. Good addition.
Note: Per our prior learning (permission checks belong in resolvers), please confirm LiveQuizService.resetAssessmentLiveQuiz does not re-introduce ownerId filters or duplicate permission checks.
apps/frontend-manage/src/components/activities/actions/useLiveQuizActions.ts (2)
9-21: Icon import looks goodfaArrowsRotate is an appropriate choice for “reset.”
48-49: New prop setResetModal — update all callsitesHook signature now requires setResetModal; update every invocation to pass it to avoid TypeScript compile errors. Repo search returned no callsites — verify and update callers in the codebase. File: apps/frontend-manage/src/components/activities/actions/useLiveQuizActions.ts (≈lines 48–49, 69–70).
packages/i18n/messages/de.ts (1)
2299-2303: LGTM – leaderboard strings consistent with cockpit copy.Text and tone align with existing strings.
packages/graphql/src/services/liveQuizzes.ts (3)
2121-2123: LGTM: clearing both results fields on cancelResetting
resultsandanonymousResultsto fresh initial values is correct and consistent.
2379-2406: Do not change to findFirst — keep findUnique with relational filtersRepository patterns show findUnique is used with relational/permission filters (examples: packages/graphql/src/services/liveQuizzes.ts:2379; packages/graphql/src/services/elements.ts:282; packages/graphql/src/services/activities.ts:369), so the suggested findUnique→findFirst change is incorrect. Optionally add isDeleted: false for consistency, but do not replace with findFirst.
Likely an incorrect or invalid review comment.
1111-1114: JWT issuer/iat added — env wired; confirm signJWT issuedAt type & centralize correlation-key
- JWT_ISSUER_API is present in CI and deployment configs and local/docker: .github/workflows/test-graphql.yml, .github/workflows/cypress-testing.yml, deploy/charts/... templates, apps/backend-docker/.env.cypress; apps/backend-docker/src/index.ts enforces the var.
- signJWT is exported from packages/util/src/jwt.ts and tests cover iat; confirm signJWT accepts a Date for issuedAt — if it doesn't, convert startedAt to a numeric iat (seconds) before calling.
- Extract a shared helper (e.g. buildCorrelationKey({ quizId, instanceId, execution, startedAt })) and use it at the two correlationKey creation sites in packages/graphql/src/services/liveQuizzes.ts.
apps/frontend-manage/src/components/courses/modals/LiveQuizResetModal.tsx (2)
64-75: LGTM: sensible defaults for confirmationsAuto-confirming zero-count items simplifies UX while still requiring explicit consent when deletions occur.
31-55: Cache update is correct — GetSingleCourseDocument expectscourseIdVerified across the repo: GetSingleCourseDocument is consistently queried with variables { courseId } (e.g. apps/frontend-manage/src/pages/courses/[id].tsx). No change required.
There was a problem hiding this comment.
Actionable comments posted: 6
♻️ Duplicate comments (4)
packages/graphql/src/services/liveQuizzes.ts (2)
2415-2426: Per-response audit logging: event flood and PII risk; aggregate per instanceEmit one audit entry per instance with {count,sumBase,sumCorrect,sumBonus}. Gate detailed logs behind a flag/sample if needed.
- for (const instance of block.elements) { - await Promise.all( - instance.liveQuizResponses.map(async (response) => { - await ctx.hatchet.events.push('create-audit-log-entry', { - info: `[INFO] [Reset Assessment Live Quiz] Deducted ${response.basePoints} base points, ${response.correctnessPoints} bonus points, and ${response.bonusPoints} bonus points from participant with ID ${response.participantId} for element instance with ID ${instance.id} in block with ID ${block.id} in live quiz with ID ${id}.`, - }) - }) - ) - } + for (const instance of block.elements) { + const agg = instance.liveQuizResponses.reduce( + (a, r) => ({ + count: a.count + 1, + sumBase: a.sumBase + (r.basePoints ?? 0), + sumCorrect: a.sumCorrect + (r.correctnessPoints ?? 0), + sumBonus: a.sumBonus + (r.bonusPoints ?? 0), + }), + { count: 0, sumBase: 0, sumCorrect: 0, sumBonus: 0 } + ) + await ctx.hatchet.events.push('create-audit-log-entry', { + info: `[INFO] [Reset Assessment Live Quiz] Deducted totals for instance ${instance.id} (block ${block.id}, quiz ${id}) -> responses=${agg.count}, base=${agg.sumBase}, correct=${agg.sumCorrect}, bonus=${agg.sumBonus}.`, + }) + }
2494-2510: Avoid crash when no direct permission found; use safe fallback
updatedQuiz.permissions[0]!may be empty; compute booleans from fallbacks.- const permission = updatedQuiz.permissions[0]! - - const { + const permissionLevel = + updatedQuiz.permissions[0]?.permissionLevel ?? DB.PermissionLevel.ADMIN + const derived = updatedQuiz.permissions[0]?.derived ?? false + const directGroupPermission = + !!updatedQuiz.permissions[0]?.directPermission?.userGroupId + + const { isOwner, isManager, isEditor, isExecutor, isShared, isRemovable, sharingType, } = getPermissionBooleans({ - permissionLevel: permission.permissionLevel, - derived: permission.derived, - directGroupPermission: - permission.directPermission && - permission.directPermission.userGroupId !== null, + permissionLevel, + derived, + directGroupPermission, }) @@ - permissionLevel: permission.permissionLevel, - derivedAccess: permission.derived, + permissionLevel, + derivedAccess: derived,Also applies to: 2531-2535
apps/auth/src/pages/api/auth/[...nextauth].ts (1)
21-25: Do not call process.exit in a Next.js API routeExiting at module load can terminate the server/runtime on import. Throw to fail-fast or validate per-request.
Apply this diff:
-// Validate required environment variables -if (!process.env.APP_ORIGIN_AUTH) { - console.error('APP_ORIGIN_AUTH is required but not defined') - process.exit(1) -} +// Validate required environment variables (fail fast without killing runtime) +if (!process.env.APP_ORIGIN_AUTH) { + throw new Error('APP_ORIGIN_AUTH is required but not defined') +}apps/backend-docker/src/index.ts (1)
121-126: Fail fast before heavy init; don’t validate envs this lateEnv validation should happen at process start (before migrate/Redis/app init) to avoid partial startup and noisy logs.
Apply these diffs:
Add near the top (after imports):
+// Validate required environment variables at startup (fail fast) +if (!process.env.APP_ORIGIN_API) { + console.error('APP_ORIGIN_API is required but not defined') + process.exit(1) +}Remove the late check:
- // Validate required environment variables at startup - if (!process.env.APP_ORIGIN_API) { - console.error('APP_ORIGIN_API is required but not defined') - process.exit(1) - }
🧹 Nitpick comments (18)
apps/auth/.env.development (2)
11-18: Avoid local/prod origin mismatch for auth issuer and callbacksYou set APP_ORIGIN_AUTH to 127.0.0.1:3010 while NEXTAUTH_URL/NEXT_PUBLIC_AUTH_URL still point to prod. This can break NextAuth callback/issuer checks in local runs. Please align or document the expected local domain setup.
Would you confirm whether local dev uses 127.0.0.1 or dev domains via hosts/tunnel? If 127.0.0.1, mirror NEXTAUTH_URL/NEXT_PUBLIC_AUTH_URL to APP_ORIGIN_AUTH in this file.
13-18: Silence dotenv-linter: drop quotes and keep alphabetical key orderNot functional, but easy to keep clean. Suggested tweak below.
-APP_ORIGIN_API="http://127.0.0.1:3000" -APP_ORIGIN_PWA="http://127.0.0.1:3001" -APP_ORIGIN_MANAGE="http://127.0.0.1:3002" -APP_ORIGIN_CONTROL="http://127.0.0.1:3003" -APP_ORIGIN_LTI="http://127.0.0.1:3005" -APP_ORIGIN_AUTH="http://127.0.0.1:3010" +APP_ORIGIN_API=http://127.0.0.1:3000 +APP_ORIGIN_AUTH=http://127.0.0.1:3010 +APP_ORIGIN_CONTROL=http://127.0.0.1:3003 +APP_ORIGIN_LTI=http://127.0.0.1:3005 +APP_ORIGIN_MANAGE=http://127.0.0.1:3002 +APP_ORIGIN_PWA=http://127.0.0.1:3001apps/frontend-pwa/.env.assessment (1)
19-19: Define APP_ORIGIN_ASSESSMENT_PWA for clarity in assessment modeRuntime code may prefer APP_ORIGIN_ASSESSMENT_PWA when ASSESSMENT mode is on. Mirror it to avoid surprises.
APP_ORIGIN_PWA=https://assessment.klicker.uzh.ch +APP_ORIGIN_ASSESSMENT_PWA=https://assessment.klicker.uzh.chPlease confirm whether getParticipantToken/createAccount select APP_ORIGIN_ASSESSMENT_PWA first in assessment mode.
apps/frontend-pwa/.env.assessment.qa (1)
19-19: QA parity: add APP_ORIGIN_ASSESSMENT_PWAKeep assessment QA consistent with prod assessment env.
APP_ORIGIN_PWA=https://assessment.klicker-qa.bf-app.ch +APP_ORIGIN_ASSESSMENT_PWA=https://assessment.klicker-qa.bf-app.chapps/frontend-manage/src/lib/apollo.ts (1)
82-84: Prefer a hard failure when no SSR API URL is configuredThe fallback chain is fine, but if all envs are missing on the server, silently passing undefined to HttpLink leads to confusing runtime errors. Fail fast.
- : process.env['API_URL_SSR'] || - process.env.NEXT_PUBLIC_API_URL_SSR || - process.env.NEXT_PUBLIC_API_URL, + : (() => { + const ssr = + process.env['API_URL_SSR'] || + process.env.NEXT_PUBLIC_API_URL_SSR || + process.env.NEXT_PUBLIC_API_URL + if (!ssr) throw new Error('Missing API_URL_SSR/NEXT_PUBLIC_API_URL[_SSR] for SSR') + return ssr + })(),deploy/charts/klicker-uzh-v2/templates/cm-frontend-manage.yaml (1)
9-10: Good: Manage SSR endpoint wiredConsistent with the Control/PWA/Assessment templates. Consider avoiding hardcoded port if a values-driven port exists in the backend chart.
If the backend service port is configurable, expose it via Values and reference it here instead of 3000.
apps/frontend-control/src/lib/apollo.ts (1)
81-84: SSR URL precedence LGTM; add a guard like in Manage to fail fastSame rationale as Manage.
- : process.env['API_URL_SSR'] || - process.env.NEXT_PUBLIC_API_URL_SSR || - process.env.NEXT_PUBLIC_API_URL, + : (() => { + const ssr = + process.env['API_URL_SSR'] || + process.env.NEXT_PUBLIC_API_URL_SSR || + process.env.NEXT_PUBLIC_API_URL + if (!ssr) throw new Error('Missing API_URL_SSR/NEXT_PUBLIC_API_URL[_SSR] for SSR') + return ssr + })(),apps/frontend-pwa/src/lib/apollo.ts (1)
99-101: SSR URL chain consistent; recommend explicit error on missing configSame pattern as Control/Manage to avoid opaque SSR failures.
- : process.env['API_URL_SSR'] || - process.env.NEXT_PUBLIC_API_URL_SSR || - process.env.NEXT_PUBLIC_API_URL, + : (() => { + const ssr = + process.env['API_URL_SSR'] || + process.env.NEXT_PUBLIC_API_URL_SSR || + process.env.NEXT_PUBLIC_API_URL + if (!ssr) throw new Error('Missing API_URL_SSR/NEXT_PUBLIC_API_URL[_SSR] for SSR') + return ssr + })(),apps/backend-docker/.env.cypress (1)
17-24: Satisfy dotenv-linter: drop quotes on URLsQuotes aren’t needed; removing them silences QuoteCharacter warnings without behavior change.
-APP_ORIGIN_AUTH="http://127.0.0.1:3010" -APP_ORIGIN_API="http://127.0.0.1:3000" -APP_ORIGIN_LTI="http://127.0.0.1:3005" -APP_ORIGIN_PWA="http://127.0.0.1:3001" -APP_ORIGIN_MANAGE="http://127.0.0.1:3002" -APP_ORIGIN_CONTROL="http://127.0.0.1:3003" -APP_ORIGIN_ASSESSMENT_API="http://127.0.0.1:3000" -APP_ORIGIN_ASSESSMENT_PWA="http://127.0.0.1:3001" +APP_ORIGIN_AUTH=http://127.0.0.1:3010 +APP_ORIGIN_API=http://127.0.0.1:3000 +APP_ORIGIN_LTI=http://127.0.0.1:3005 +APP_ORIGIN_PWA=http://127.0.0.1:3001 +APP_ORIGIN_MANAGE=http://127.0.0.1:3002 +APP_ORIGIN_CONTROL=http://127.0.0.1:3003 +APP_ORIGIN_ASSESSMENT_API=http://127.0.0.1:3000 +APP_ORIGIN_ASSESSMENT_PWA=http://127.0.0.1:3001packages/graphql/src/services/liveQuizzes.ts (1)
2373-2553: Optional: clear stale Redis keys when resetting ended quizIf any cache survived end/aggregation, wipe
lq:${id}:*to avoid interference on next run.@@ await ctx.hatchet.events.push('create-audit-log-entry', { info: `[INFO] [Reset Assessment Live Quiz] Successfully reset assessment live quiz with ID ${id}.`, }) ctx.emitter.emit('invalidate', { typename: 'LiveQuiz', id }) + // best-effort cache cleanup + try { + const keys = await ctx.redisExec.keys(`lq:${id}:*`) + if (keys.length) { + const pipe = ctx.redisExec.pipeline() + keys.forEach((k) => pipe.unlink(k)) + await pipe.exec() + } + } catch (_) {}packages/graphql/src/services/groups.ts (1)
402-402: Build absolute URL robustly; fail fast if APP_ORIGIN_MANAGE is missingUse URL(base, origin) to normalize slashes and require the origin at runtime.
- const courseGroupsOverviewLink = `${process.env.APP_ORIGIN_MANAGE}/courses/${course.id}?gamificationTab=groups` + const courseGroupsOverviewLink = new URL( + `/courses/${course.id}?gamificationTab=groups`, + process.env.APP_ORIGIN_MANAGE! + ).toString()Verified: deploy/charts/klicker-uzh-v2/templates/cm-global.yaml exports APP_ORIGIN_MANAGE; it’s also set in local .env and CI workflows.
apps/auth/src/pages/api/auth/[...nextauth].ts (1)
627-628: Hardcoded scope literalIf scope values are enumerated elsewhere, prefer a constant/enum to avoid typos and ease refactors.
- token.scope = 'EDUID' + token.scope = 'EDUID' // TODO: consider centralizing as a constant/enumapps/backend-docker/src/index.ts (1)
81-81: Type the invalidate payloadAvoid any; define the minimal shape for clarity and safety.
-emitter.on('invalidate', (resource: any) => { +type InvalidateResource = { typename: string; id: string } +emitter.on('invalidate', (resource: InvalidateResource) => {apps/frontend-pwa/src/pages/createAccount.tsx (1)
146-155: Deduplicate issuer origin and fix misleading error textYou compute
pwaOriginbut don't use it; and the error implies both vars are required. ReusepwaOriginas the issuer and clarify the message.- const pwaOrigin = - process.env.ASSESSMENT_MODE === 'true' - ? process.env.APP_ORIGIN_ASSESSMENT_PWA - : process.env.APP_ORIGIN_PWA - if (!pwaOrigin) { - throw new Error( - 'APP_ORIGIN_PWA and APP_ORIGIN_ASSESSMENT_PWA are required but not defined' - ) - } + const assessmentMode = process.env.ASSESSMENT_MODE === 'true' + const pwaOrigin = assessmentMode + ? process.env.APP_ORIGIN_ASSESSMENT_PWA || process.env.APP_ORIGIN_PWA + : process.env.APP_ORIGIN_PWA + if (!pwaOrigin) { + throw new Error( + assessmentMode + ? 'APP_ORIGIN_ASSESSMENT_PWA or APP_ORIGIN_PWA must be defined' + : 'APP_ORIGIN_PWA must be defined' + ) + } @@ - issuer: - process.env.ASSESSMENT_MODE === 'true' - ? process.env.APP_ORIGIN_ASSESSMENT_PWA || - process.env.APP_ORIGIN_PWA - : process.env.APP_ORIGIN_PWA, + issuer: pwaOrigin,Also ensure LTI 1.3 verification (Lines 120–127) intentionally skips issuer checking, or pass the expected issuer there if required.
Also applies to: 166-171
apps/backend-docker/.env.example (1)
16-20: Silence dotenv-linter nits (quotes/order) and confirm PWA port consistencyUnquote values and order keys to satisfy linter; verify whether PWA should be 3001 (Cypress) or 3002 (this example).
-APP_ORIGIN_AUTH="http://127.0.0.1:3010" -APP_ORIGIN_API="http://127.0.0.1:3000" -APP_ORIGIN_LTI="http://127.0.0.1:3005" -APP_ORIGIN_PWA="http://127.0.0.1:3002" +APP_ORIGIN_API=http://127.0.0.1:3000 +APP_ORIGIN_AUTH=http://127.0.0.1:3010 +APP_ORIGIN_LTI=http://127.0.0.1:3005 +APP_ORIGIN_PWA=http://127.0.0.1:3002deploy/charts/klicker-uzh-v2/templates/cm-global.yaml (1)
1-24: Remove unused local Helm var
$fullNameis declared but unused. Drop it to avoid noise; YAMLlint error is a known false positive on Helm directives.-{{- $fullName := include "chart.fullname" . -}} apiVersion: v1 kind: ConfigMap metadata: name: {{ include "chart.fullname" . }}-config-global.github/workflows/cypress-testing.yml (2)
202-211: Expose APP_ORIGIN_ to Cypress via CYPRESS_ prefix*If specs use
Cypress.env('APP_ORIGIN_API'), duplicate withCYPRESS_so the browser process can read them.# Ensure Cypress process has all origins, too APP_ORIGIN_API: http://127.0.0.1:3000 APP_ORIGIN_AUTH: http://127.0.0.1:3010 APP_ORIGIN_LTI: http://127.0.0.1:3005 APP_ORIGIN_PWA: http://127.0.0.1:3001 APP_ORIGIN_MANAGE: http://127.0.0.1:3002 APP_ORIGIN_CONTROL: http://127.0.0.1:3003 APP_ORIGIN_ASSESSMENT_API: http://127.0.0.1:3000 APP_ORIGIN_ASSESSMENT_PWA: http://127.0.0.1:3001 + CYPRESS_APP_ORIGIN_API: http://127.0.0.1:3000 + CYPRESS_APP_ORIGIN_AUTH: http://127.0.0.1:3010 + CYPRESS_APP_ORIGIN_LTI: http://127.0.0.1:3005 + CYPRESS_APP_ORIGIN_PWA: http://127.0.0.1:3001 + CYPRESS_APP_ORIGIN_MANAGE: http://127.0.0.1:3002 + CYPRESS_APP_ORIGIN_CONTROL: http://127.0.0.1:3003 + CYPRESS_APP_ORIGIN_ASSESSMENT_API: http://127.0.0.1:3000 + CYPRESS_APP_ORIGIN_ASSESSMENT_PWA: http://127.0.0.1:3001
407-415: Expose APP_ORIGIN_ to Cypress via CYPRESS_ prefix (Cloud job)*Same duplication needed here for
Cypress.env(...).# Ensure Cypress process has all origins, too APP_ORIGIN_API: http://127.0.0.1:3000 APP_ORIGIN_AUTH: http://127.0.0.1:3010 APP_ORIGIN_LTI: http://127.0.0.1:3005 APP_ORIGIN_PWA: http://127.0.0.1:3001 APP_ORIGIN_MANAGE: http://127.0.0.1:3002 APP_ORIGIN_CONTROL: http://127.0.0.1:3003 APP_ORIGIN_ASSESSMENT_API: http://127.0.0.1:3000 APP_ORIGIN_ASSESSMENT_PWA: http://127.0.0.1:3001 + CYPRESS_APP_ORIGIN_API: http://127.0.0.1:3000 + CYPRESS_APP_ORIGIN_AUTH: http://127.0.0.1:3010 + CYPRESS_APP_ORIGIN_LTI: http://127.0.0.1:3005 + CYPRESS_APP_ORIGIN_PWA: http://127.0.0.1:3001 + CYPRESS_APP_ORIGIN_MANAGE: http://127.0.0.1:3002 + CYPRESS_APP_ORIGIN_CONTROL: http://127.0.0.1:3003 + CYPRESS_APP_ORIGIN_ASSESSMENT_API: http://127.0.0.1:3000 + CYPRESS_APP_ORIGIN_ASSESSMENT_PWA: http://127.0.0.1:3001
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (37)
.github/workflows/cypress-testing.yml(4 hunks).github/workflows/test-graphql.yml(2 hunks)apps/auth/.env.development(1 hunks)apps/auth/src/pages/api/auth/[...nextauth].ts(3 hunks)apps/backend-docker/.env.cypress(1 hunks)apps/backend-docker/.env.example(1 hunks)apps/backend-docker/src/index.ts(2 hunks)apps/frontend-control/src/lib/apollo.ts(1 hunks)apps/frontend-manage/src/lib/apollo.ts(1 hunks)apps/frontend-pwa/.env.assessment(1 hunks)apps/frontend-pwa/.env.assessment.qa(1 hunks)apps/frontend-pwa/.env.development(1 hunks)apps/frontend-pwa/.env.production(1 hunks)apps/frontend-pwa/.env.qa(1 hunks)apps/frontend-pwa/.env.test(1 hunks)apps/frontend-pwa/src/lib/apollo.ts(1 hunks)apps/frontend-pwa/src/lib/getParticipantToken.ts(2 hunks)apps/frontend-pwa/src/pages/createAccount.tsx(2 hunks)apps/lti/src/index.ts(3 hunks)cypress/cypress/support/commands.ts(2 hunks)deploy/charts/klicker-uzh-v2/templates/cm-frontend-assessment.yaml(1 hunks)deploy/charts/klicker-uzh-v2/templates/cm-frontend-control.yaml(1 hunks)deploy/charts/klicker-uzh-v2/templates/cm-frontend-manage.yaml(1 hunks)deploy/charts/klicker-uzh-v2/templates/cm-frontend-pwa.yaml(1 hunks)deploy/charts/klicker-uzh-v2/templates/cm-global.yaml(1 hunks)deploy/charts/klicker-uzh-v2/templates/deployment-app.yaml(7 hunks)deploy/charts/klicker-uzh-v2/templates/deployment-assessment.yaml(2 hunks)deploy/charts/klicker-uzh-v2/templates/deployment-hatchet-workers.yaml(3 hunks)deploy/charts/klicker-uzh-v2/templates/deployment-response-api.yaml(2 hunks)deploy/charts/klicker-uzh-v2/values.yaml(1 hunks)deploy/env-prod-v3/values.yaml(1 hunks)deploy/env-qa-v3/values.yaml(1 hunks)packages/graphql/src/scripts/impersonateParticipant.ts(1 hunks)packages/graphql/src/scripts/impersonateUser.ts(1 hunks)packages/graphql/src/services/accounts.ts(4 hunks)packages/graphql/src/services/groups.ts(1 hunks)packages/graphql/src/services/liveQuizzes.ts(4 hunks)
🚧 Files skipped from review as they are similar to previous changes (12)
- cypress/cypress/support/commands.ts
- deploy/env-prod-v3/values.yaml
- deploy/charts/klicker-uzh-v2/templates/cm-frontend-pwa.yaml
- deploy/env-qa-v3/values.yaml
- apps/frontend-pwa/.env.qa
- packages/graphql/src/services/accounts.ts
- deploy/charts/klicker-uzh-v2/values.yaml
- packages/graphql/src/scripts/impersonateParticipant.ts
- apps/frontend-pwa/.env.development
- packages/graphql/src/scripts/impersonateUser.ts
- .github/workflows/test-graphql.yml
- apps/frontend-pwa/.env.test
🧰 Additional context used
🧠 Learnings (5)
📚 Learning: 2025-09-10T22:07:32.100Z
Learnt from: sjschlapbach
PR: uzh-bf/klicker-uzh#4893
File: apps/frontend-pwa/.env.assessment:6-6
Timestamp: 2025-09-10T22:07:32.100Z
Learning: The Klicker UZH application uses a multi-tenant deployment architecture where the same codebase is deployed on separate pods for assessment mode and standard mode. During development, different commands start the application in either assessment or standard mode. During deployment, an assessment flag is used to create separate containers - one set for assessment frontend (assessment.klicker.com) and response API (assessment-response-api.com), and another set for standard mode. Traefik routing configuration is handled at the deployment/infrastructure level rather than in the repository's util/traefik directory.
Applied to files:
apps/frontend-pwa/.env.assessment.qaapps/frontend-pwa/.env.assessment
📚 Learning: 2025-06-23T12:32:13.205Z
Learnt from: CR
PR: uzh-bf/klicker-uzh#0
File: apps/backend-docker/CLAUDE.md:0-0
Timestamp: 2025-06-23T12:32:13.205Z
Learning: Authentication is implemented using Passport.js with JWT strategy, extracting tokens from specific cookies or the Authorization header, and verifying them with the APP_SECRET environment variable.
Applied to files:
apps/auth/src/pages/api/auth/[...nextauth].ts
📚 Learning: 2025-06-23T12:32:13.205Z
Learnt from: CR
PR: uzh-bf/klicker-uzh#0
File: apps/backend-docker/CLAUDE.md:0-0
Timestamp: 2025-06-23T12:32:13.205Z
Learning: The GraphQL server uses GraphQL Yoga with plugins for security (GraphQL Armor), CSRF prevention, persisted operations, and optional response caching.
Applied to files:
apps/backend-docker/src/index.ts
📚 Learning: 2025-04-17T12:52:17.002Z
Learnt from: sjschlapbach
PR: uzh-bf/klicker-uzh#4606
File: packages/graphql/src/services/liveQuizzes.ts:1100-1106
Timestamp: 2025-04-17T12:52:17.002Z
Learning: In the application architecture, permission checks for operations like activateLiveQuizBlock, deactivateLiveQuizBlock, endLiveQuiz, cancelLiveQuiz, and deleteLiveQuiz are performed at the GraphQL mutation resolver level using the checkAccess function, rather than in the service functions themselves. This separation of concerns means that ownerId filters within service functions are no longer necessary.
Applied to files:
packages/graphql/src/services/liveQuizzes.ts
📚 Learning: 2025-09-08T19:16:04.791Z
Learnt from: sjschlapbach
PR: uzh-bf/klicker-uzh#4884
File: apps/frontend-pwa/src/components/liveQuiz/LiveQuizSubscriber.tsx:21-37
Timestamp: 2025-09-08T19:16:04.791Z
Learning: For RunningLiveQuizUpdatedDocument subscription in apps/frontend-pwa/src/components/liveQuiz/LiveQuizSubscriber.tsx, the backend implementation guarantees non-null values, so frontend null checks would be unnecessary and falsify return types. The subscription schema should be updated to reflect non-nullable return types instead.
Applied to files:
packages/graphql/src/services/liveQuizzes.ts
🧬 Code graph analysis (2)
apps/auth/src/pages/api/auth/[...nextauth].ts (1)
packages/util/src/jwt.ts (2)
signJWT(19-43)JWTPayload(3-12)
packages/graphql/src/services/liveQuizzes.ts (3)
packages/graphql/src/lib/context.ts (1)
ContextWithUser(37-46)packages/util/src/elements.ts (1)
getInitialInstanceResults(185-297)packages/graphql/src/services/activities.ts (1)
getPermissionBooleans(55-86)
🪛 dotenv-linter (3.3.0)
apps/auth/.env.development
[warning] 13-13: [QuoteCharacter] The value has quote characters (', ")
(QuoteCharacter)
[warning] 14-14: [QuoteCharacter] The value has quote characters (', ")
(QuoteCharacter)
[warning] 15-15: [QuoteCharacter] The value has quote characters (', ")
(QuoteCharacter)
[warning] 15-15: [UnorderedKey] The APP_ORIGIN_MANAGE key should go before the APP_ORIGIN_PWA key
(UnorderedKey)
[warning] 16-16: [QuoteCharacter] The value has quote characters (', ")
(QuoteCharacter)
[warning] 16-16: [UnorderedKey] The APP_ORIGIN_CONTROL key should go before the APP_ORIGIN_MANAGE key
(UnorderedKey)
[warning] 17-17: [QuoteCharacter] The value has quote characters (', ")
(QuoteCharacter)
[warning] 17-17: [UnorderedKey] The APP_ORIGIN_LTI key should go before the APP_ORIGIN_MANAGE key
(UnorderedKey)
[warning] 18-18: [QuoteCharacter] The value has quote characters (', ")
(QuoteCharacter)
[warning] 18-18: [UnorderedKey] The APP_ORIGIN_AUTH key should go before the APP_ORIGIN_CONTROL key
(UnorderedKey)
apps/backend-docker/.env.example
[warning] 17-17: [QuoteCharacter] The value has quote characters (', ")
(QuoteCharacter)
[warning] 18-18: [QuoteCharacter] The value has quote characters (', ")
(QuoteCharacter)
[warning] 18-18: [UnorderedKey] The APP_ORIGIN_API key should go before the APP_ORIGIN_AUTH key
(UnorderedKey)
[warning] 19-19: [QuoteCharacter] The value has quote characters (', ")
(QuoteCharacter)
[warning] 20-20: [QuoteCharacter] The value has quote characters (', ")
(QuoteCharacter)
apps/backend-docker/.env.cypress
[warning] 17-17: [QuoteCharacter] The value has quote characters (', ")
(QuoteCharacter)
[warning] 18-18: [QuoteCharacter] The value has quote characters (', ")
(QuoteCharacter)
[warning] 18-18: [UnorderedKey] The APP_ORIGIN_API key should go before the APP_ORIGIN_AUTH key
(UnorderedKey)
[warning] 19-19: [QuoteCharacter] The value has quote characters (', ")
(QuoteCharacter)
[warning] 20-20: [QuoteCharacter] The value has quote characters (', ")
(QuoteCharacter)
[warning] 21-21: [QuoteCharacter] The value has quote characters (', ")
(QuoteCharacter)
[warning] 21-21: [UnorderedKey] The APP_ORIGIN_MANAGE key should go before the APP_ORIGIN_PWA key
(UnorderedKey)
[warning] 22-22: [QuoteCharacter] The value has quote characters (', ")
(QuoteCharacter)
[warning] 22-22: [UnorderedKey] The APP_ORIGIN_CONTROL key should go before the APP_ORIGIN_LTI key
(UnorderedKey)
[warning] 23-23: [QuoteCharacter] The value has quote characters (', ")
(QuoteCharacter)
[warning] 23-23: [UnorderedKey] The APP_ORIGIN_ASSESSMENT_API key should go before the APP_ORIGIN_AUTH key
(UnorderedKey)
[warning] 24-24: [QuoteCharacter] The value has quote characters (', ")
(QuoteCharacter)
[warning] 24-24: [UnorderedKey] The APP_ORIGIN_ASSESSMENT_PWA key should go before the APP_ORIGIN_AUTH key
(UnorderedKey)
🪛 YAMLlint (1.37.1)
deploy/charts/klicker-uzh-v2/templates/cm-global.yaml
[error] 1-1: syntax error: expected the node content, but found '-'
(syntax)
🔇 Additional comments (21)
deploy/charts/klicker-uzh-v2/templates/cm-frontend-control.yaml (1)
9-10: Good: SSR GraphQL endpoint provided via ConfigMapAPI_URL_SSR unblocks server-side Apollo calls without exposing internal DNS to the client. Looks right.
Double-check the backend service listens on port 3000 in this chart and that the Control deployment mounts this ConfigMap (envFrom) so API_URL_SSR is present at runtime.
packages/graphql/src/services/liveQuizzes.ts (3)
2121-2123: LGTM: results reset is correctResetting results and anonymousResults to fresh initial values is right for cancel.
2379-2395: Prisma: findUnique cannot filter by relations; use findFirst or split checksRelational filters under
course.permissionsaren’t valid onfindUnique. This will fail type-check/runtime.- const liveQuiz = await ctx.prisma.liveQuiz.findUnique({ - where: { - id, - isAssessmentEnabled: true, - status: DB.PublicationStatus.ENDED, - course: { - permissions: { some: { userId: ctx.user.sub, permissionLevel: { in: [DB.PermissionLevel.ADMIN, DB.PermissionLevel.OWNER] } } }, - }, - }, + const liveQuiz = await ctx.prisma.liveQuiz.findFirst({ + where: { + id, + isAssessmentEnabled: true, + status: DB.PublicationStatus.ENDED, + course: { + permissions: { some: { userId: ctx.user.sub, permissionLevel: { in: [DB.PermissionLevel.ADMIN, DB.PermissionLevel.OWNER] } } }, + }, + },⛔ Skipped due to learnings
Learnt from: sjschlapbach PR: uzh-bf/klicker-uzh#4624 File: packages/graphql/src/services/feedbacks.ts:298-304 Timestamp: 2025-04-17T10:22:49.381Z Learning: In the klicker-uzh project, relation filters like `where: { id, feedback: { liveQuizId } }` work with Prisma's `findUnique` method despite documentation suggesting otherwise.Learnt from: sjschlapbach PR: uzh-bf/klicker-uzh#4606 File: packages/graphql/src/services/questions.ts:134-169 Timestamp: 2025-04-17T18:03:56.094Z Learning: In the klicker-uzh project, complex relational filters with OR conditions can be used with Prisma's `findUnique` method even though standard Prisma documentation would suggest using `findFirst` instead.Learnt from: sjschlapbach PR: uzh-bf/klicker-uzh#4606 File: packages/graphql/src/services/liveQuizzes.ts:1100-1106 Timestamp: 2025-04-17T12:52:17.002Z Learning: In the application architecture, permission checks for operations like activateLiveQuizBlock, deactivateLiveQuizBlock, endLiveQuiz, cancelLiveQuiz, and deleteLiveQuiz are performed at the GraphQL mutation resolver level using the checkAccess function, rather than in the service functions themselves. This separation of concerns means that ownerId filters within service functions are no longer necessary.Learnt from: sjschlapbach PR: uzh-bf/klicker-uzh#4625 File: packages/graphql/src/services/liveQuizzes.ts:771-779 Timestamp: 2025-04-17T12:47:09.182Z Learning: In the KlickerUZH system, running live quizzes (with status PublicationStatus.PUBLISHED) cannot be deleted as prevented by validation logic in the deleteLiveQuiz function, making the isDeleted: false filter redundant when querying for published quizzes.
1112-1114: JWT: add issuer fallback and safer iat; put scope in the payload (signJWT doesn't accept scope)
- Change issuer to process.env.APP_ORIGIN_ASSESSMENT_API ?? process.env.APP_ORIGIN_API and issuedAt to updatedQuiz.activeBlock?.startedAt ?? new Date().
- Do not pass scope in signJWT's options — signJWT supports issuer and issuedAt but not scope; either add scope: 'assessment:response:submit' to the JWT payload or extend signJWT/options+implementation to accept and set scope.
Likely an incorrect or invalid review comment.
apps/frontend-pwa/.env.production (1)
21-21: LGTM: adds canonical PWA originMatches the issuer/origin wiring elsewhere. No issues.
deploy/charts/klicker-uzh-v2/templates/cm-frontend-assessment.yaml (1)
9-10: SSR GraphQL URL for Assessment PWA: OKConsistent with other frontends using server-only API_URL_SSR.
Confirm the Assessment frontend reads API_URL_SSR (server-side) and not NEXT_PUBLIC_API_URL_SSR for SSR requests.
deploy/charts/klicker-uzh-v2/templates/deployment-app.yaml (7)
51-52: Global config env injection: OK for authOrder allows per-service ConfigMap to override globals if needed. Watch for key collisions.
Ensure no duplicate keys between config-global and config-auth unless override is intentional.
130-132: Global config env injection: OK for frontend-pwaSame note on potential key overlap.
Confirm APP_ORIGIN_* are present in the global ConfigMap for this environment.
209-211: Global config env injection: OK for frontend-manageLooks good.
288-290: Global config env injection: OK for frontend-controlLooks good.
370-372: Global config env injection: OK for backend-graphqlConsider validating required APP_ORIGIN_* at process start (see backend-docker comment).
Add a startup env schema check to fail fast in this service as well if not already present.
452-454: Global config env injection: OK for olat-apiLooks good.
533-535: Global config env injection: OK for LTILooks good.
apps/auth/src/pages/api/auth/[...nextauth].ts (1)
140-143: Issuer included in NextAuth JWT encoding: OKGood to embed issuer using APP_ORIGIN_AUTH.
apps/lti/src/index.ts (2)
6-11: Boot-time env validation: OKFail-fast before heavy initialization is fine for this standalone service.
79-80: Issuer added to LTI JWTs: OKCorrect issuer binding to APP_ORIGIN_LTI.
deploy/charts/klicker-uzh-v2/templates/deployment-hatchet-workers.yaml (3)
47-49: Global config env injection: OK (general worker)Consistent with app deployments. Watch for key collisions.
Ensure component-specific ConfigMap does not redefine APP_ORIGIN_* unintentionally.
125-127: Global config env injection: OK (response-processor)Looks good.
203-205: Global config env injection: OK (response-processor-assessment)Looks good.
.github/workflows/cypress-testing.yml (2)
174-183: LGTM: origins provided to servicesGood: centralizes all APP_ORIGIN_* for service startup/readiness.
366-375: LGTM: origins provided to services (Cloud job)Mirrors the draft job; consistent.
| const pwaOrigin = | ||
| process.env.ASSESSMENT_MODE === 'true' | ||
| ? process.env.APP_ORIGIN_ASSESSMENT_PWA | ||
| : process.env.APP_ORIGIN_PWA | ||
| if (!pwaOrigin) { | ||
| throw new Error( | ||
| 'APP_ORIGIN_PWA and APP_ORIGIN_ASSESSMENT_PWA are required but not defined' | ||
| ) | ||
| } | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion
Unify origin derivation and improve error text
Compute once and reuse. Clarify that only the relevant var is required depending on ASSESSMENT_MODE.
- const pwaOrigin =
- process.env.ASSESSMENT_MODE === 'true'
- ? process.env.APP_ORIGIN_ASSESSMENT_PWA
- : process.env.APP_ORIGIN_PWA
- if (!pwaOrigin) {
- throw new Error(
- 'APP_ORIGIN_PWA and APP_ORIGIN_ASSESSMENT_PWA are required but not defined'
- )
- }
+ const pwaOrigin =
+ process.env.ASSESSMENT_MODE === 'true'
+ ? process.env.APP_ORIGIN_ASSESSMENT_PWA
+ : process.env.APP_ORIGIN_PWA
+ if (!pwaOrigin) {
+ throw new Error(
+ 'PWA origin is required: set APP_ORIGIN_PWA (and APP_ORIGIN_ASSESSMENT_PWA when ASSESSMENT_MODE=true)'
+ )
+ }📝 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.
| const pwaOrigin = | |
| process.env.ASSESSMENT_MODE === 'true' | |
| ? process.env.APP_ORIGIN_ASSESSMENT_PWA | |
| : process.env.APP_ORIGIN_PWA | |
| if (!pwaOrigin) { | |
| throw new Error( | |
| 'APP_ORIGIN_PWA and APP_ORIGIN_ASSESSMENT_PWA are required but not defined' | |
| ) | |
| } | |
| const pwaOrigin = | |
| process.env.ASSESSMENT_MODE === 'true' | |
| ? process.env.APP_ORIGIN_ASSESSMENT_PWA | |
| : process.env.APP_ORIGIN_PWA | |
| if (!pwaOrigin) { | |
| throw new Error( | |
| 'PWA origin is required: set APP_ORIGIN_PWA (and APP_ORIGIN_ASSESSMENT_PWA when ASSESSMENT_MODE=true)' | |
| ) | |
| } |
| issuer: | ||
| process.env.ASSESSMENT_MODE === 'true' | ||
| ? process.env.APP_ORIGIN_ASSESSMENT_PWA | ||
| : process.env.APP_ORIGIN_PWA, | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
❓ Verification inconclusive
Use the computed origin for issuer
Avoid diverging reads from env; reuse pwaOrigin.
- issuer:
- process.env.ASSESSMENT_MODE === 'true'
- ? process.env.APP_ORIGIN_ASSESSMENT_PWA
- : process.env.APP_ORIGIN_PWA,
+ issuer: pwaOrigin,Optionally, also assert issuer on LTI1.3 tokens after verifyJWT by checking decoded.iss against APP_ORIGIN_LTI.
Use the computed origin for issuer
Avoid diverging reads from env; reuse pwaOrigin.
- issuer:
- process.env.ASSESSMENT_MODE === 'true'
- ? process.env.APP_ORIGIN_ASSESSMENT_PWA
- : process.env.APP_ORIGIN_PWA,
+ issuer: pwaOrigin,Optionally, also assert issuer on LTI1.3 tokens after verifyJWT by checking decoded.iss against APP_ORIGIN_LTI.
📝 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.
| issuer: | |
| process.env.ASSESSMENT_MODE === 'true' | |
| ? process.env.APP_ORIGIN_ASSESSMENT_PWA | |
| : process.env.APP_ORIGIN_PWA, | |
| } | |
| issuer: pwaOrigin, | |
| } |
🤖 Prompt for AI Agents
In apps/frontend-pwa/src/lib/getParticipantToken.ts around lines 111 to 115, the
code re-reads environment variables for issuer instead of using the
already-computed pwaOrigin; change the issuer value to reuse the pwaOrigin
variable (i.e., set issuer: pwaOrigin) to avoid divergent reads, and optionally
after verifyJWT add an assertion that for LTI1.3 tokens decoded.iss ===
process.env.APP_ORIGIN_LTI (or fail/throw) to ensure the token issuer is as
expected.
| if (!process.env.APP_ORIGIN_LTI) { | ||
| console.error('APP_ORIGIN_LTI is required but not defined') | ||
| process.exit(1) | ||
| } | ||
|
|
There was a problem hiding this comment.
Never process.exit() inside a request/connection handler
Killing the process on a single request leads to outages. Respond with 5xx instead and keep running.
Apply this diff (or remove the duplicate check entirely since it already happens at startup):
- if (!process.env.APP_ORIGIN_LTI) {
- console.error('APP_ORIGIN_LTI is required but not defined')
- process.exit(1)
- }
+ if (!process.env.APP_ORIGIN_LTI) {
+ console.error('APP_ORIGIN_LTI is required but not defined')
+ res.status(500).send('Server misconfigured')
+ return
+ }Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In apps/lti/src/index.ts around lines 64-68, the code calls process.exit(1) when
APP_ORIGIN_LTI is missing; instead remove the process.exit() and either (A)
delete this duplicate runtime check entirely if the environment is already
validated at startup, or (B) if you must guard here (inside a request/connection
handler), log the error and return/throw an HTTP 5xx error to the caller so the
server stays running (e.g., set an appropriate 500 response or propagate an
error to your middleware) rather than terminating the process.
| - configMapRef: | ||
| name: {{ include "chart.fullname" . }}-config-global | ||
| - configMapRef: |
There was a problem hiding this comment.
Config map checksum missing for config-global; pods won’t roll on APP_ORIGIN changes
Add cm-global to checksum annotations in both Deployments so env changes trigger rollout.
@@
annotations:
- checksum/config: {{ include (print $.Template.BasePath "/cm-frontend-assessment.yaml") . | sha256sum }}
+ checksum/config: {{ include (print $.Template.BasePath "/cm-frontend-assessment.yaml") . | sha256sum }}
+ checksum/config-global: {{ include (print $.Template.BasePath "/cm-global.yaml") . | sha256sum }}
@@
annotations:
- checksum/config: {{ include (print $.Template.BasePath "/cm-backend-assessment.yaml") . | sha256sum }}
+ checksum/config: {{ include (print $.Template.BasePath "/cm-backend-assessment.yaml") . | sha256sum }}
+ checksum/config-global: {{ include (print $.Template.BasePath "/cm-global.yaml") . | sha256sum }}📝 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.
| - configMapRef: | |
| name: {{ include "chart.fullname" . }}-config-global | |
| - configMapRef: | |
| annotations: | |
| checksum/config: {{ include (print $.Template.BasePath "/cm-frontend-assessment.yaml") . | sha256sum }} | |
| checksum/config-global: {{ include (print $.Template.BasePath "/cm-global.yaml") . | sha256sum }} | |
| @@ | |
| annotations: | |
| checksum/config: {{ include (print $.Template.BasePath "/cm-backend-assessment.yaml") . | sha256sum }} | |
| checksum/config-global: {{ include (print $.Template.BasePath "/cm-global.yaml") . | sha256sum }} |
🤖 Prompt for AI Agents
In deploy/charts/klicker-uzh-v2/templates/deployment-assessment.yaml around
lines 51–53, the deployment is referencing the config-global ConfigMap but there
is no checksum annotation for it, so pods won’t roll when APP_ORIGIN (or other
config-global data) changes; add a checksum annotation for the config-global
ConfigMap in the deployment metadata.annotations (same pattern used for other
configMap checksums) so Helm will trigger a rollout on changes, and do the same
update in the other Deployment template that manages app pods.
| - configMapRef: | ||
| name: {{ include "chart.fullname" . }}-config-global | ||
| - configMapRef: |
There was a problem hiding this comment.
Add checksum for config-global to trigger rollout on origin changes
Same rationale as assessment deployments.
@@
annotations:
- checksum/config: {{ include (print $.Template.BasePath "/cm-response-api.yaml") . | sha256sum }}
+ checksum/config: {{ include (print $.Template.BasePath "/cm-response-api.yaml") . | sha256sum }}
+ checksum/config-global: {{ include (print $.Template.BasePath "/cm-global.yaml") . | sha256sum }}
@@
annotations:
- checksum/config: {{ include (print $.Template.BasePath "/cm-response-api-assessment.yaml") . | sha256sum }}
+ checksum/config: {{ include (print $.Template.BasePath "/cm-response-api-assessment.yaml") . | sha256sum }}
+ checksum/config-global: {{ include (print $.Template.BasePath "/cm-global.yaml") . | sha256sum }}Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In deploy/charts/klicker-uzh-v2/templates/deployment-response-api.yaml around
lines 54-56, the pod spec references the configMapRef for {{ include
"chart.fullname" . }}-config-global but lacks a checksum annotation to trigger
rollouts when that ConfigMap changes; add an annotation on the Pod template
metadata such as checksum/config-global whose value is the sha256 (or sprig
sha256sum) of the rendered config-global ConfigMap (use the same
include/template helper used for assessment deployments) so Kubernetes will
create a new ReplicaSet when the ConfigMap content changes.
| issuer: process.env.APP_ORIGIN_ASSESSMENT_API, | ||
| issuedAt: quiz.activeBlock?.startedAt ?? new Date(0), | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Duplicate JWT fix for student path
Mirror issuer/scope/iat handling here for consistency.
- {
- issuer: process.env.APP_ORIGIN_ASSESSMENT_API,
- issuedAt: quiz.activeBlock?.startedAt ?? new Date(0),
- }
+ {
+ issuer:
+ process.env.APP_ORIGIN_ASSESSMENT_API ??
+ process.env.APP_ORIGIN_API,
+ issuedAt: quiz.activeBlock?.startedAt ?? new Date(),
+ scope: 'assessment:response:submit',
+ }📝 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.
| issuer: process.env.APP_ORIGIN_ASSESSMENT_API, | |
| issuedAt: quiz.activeBlock?.startedAt ?? new Date(0), | |
| } | |
| issuer: | |
| process.env.APP_ORIGIN_ASSESSMENT_API ?? | |
| process.env.APP_ORIGIN_API, | |
| issuedAt: quiz.activeBlock?.startedAt ?? new Date(), | |
| scope: 'assessment:response:submit', | |
| } |
🤖 Prompt for AI Agents
In packages/graphql/src/services/liveQuizzes.ts around lines 2829-2831, the
student JWT path needs the same issuer, scope, and issuedAt handling as the
other path; update the student token claims to set issuer to
process.env.APP_ORIGIN_ASSESSMENT_API, include the same scope claim used
elsewhere, and set issuedAt to quiz.activeBlock?.startedAt ?? new Date(0) so
both paths produce consistent JWTs.
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
cypress/cypress/support/commands.ts (1)
142-149: Use Cypress.env for APP_ORIGIN_AUTH in Cypress runtime (critical)process.env is used at cypress/cypress/support/commands.ts (lines 142–149) and can be undefined in browser-run Cypress tests — read APP_ORIGIN_AUTH via Cypress.env, guard it, and pass a non-empty issuer to setIssuer.
cy.wrap(null).then(async () => { + const issuer = Cypress.env('APP_ORIGIN_AUTH') + if (!issuer) { + throw new Error('APP_ORIGIN_AUTH is not set in Cypress env (expected for JWT issuer)') + } const token = await new jose.SignJWT(tokenData) .setProtectedHeader({ alg }) .setIssuedAt() .setExpirationTime('2h') - .setIssuer(process.env.APP_ORIGIN_AUTH) + .setIssuer(issuer) .sign(secret)Ensure CI/local config exposes it to Cypress (either prefix env var as CYPRESS_APP_ORIGIN_AUTH or map via cypress.config):
// cypress.config.(ts|js) env: { APP_ORIGIN_AUTH: process.env.APP_ORIGIN_AUTH, }Run to check for any remaining process.env usage in Cypress code:
rg -nC2 'process\.env\.' cypress
🧹 Nitpick comments (19)
apps/response-api/.env.example (2)
7-14: Drop quotes and alphabetize APP_ORIGIN_ keys to satisfy dotenv-linter and keep style consistent.*Quotes aren’t needed here and trigger linter warnings; ordering the keys alphabetically will also clear UnorderedKey warnings.
Apply this diff:
-APP_ORIGIN_AUTH="http://127.0.0.1:3010" -APP_ORIGIN_API="http://127.0.0.1:3000" -APP_ORIGIN_LTI="http://127.0.0.1:4000" -APP_ORIGIN_PWA="http://127.0.0.1:3001" -APP_ORIGIN_MANAGE="http://127.0.0.1:3002" -APP_ORIGIN_CONTROL="http://127.0.0.1:3003" -APP_ORIGIN_ASSESSMENT_API="http://127.0.0.1:3000" -APP_ORIGIN_ASSESSMENT_PWA="http://127.0.0.1:3001" +APP_ORIGIN_API=http://127.0.0.1:3000 +APP_ORIGIN_ASSESSMENT_API=http://127.0.0.1:3000 +APP_ORIGIN_ASSESSMENT_PWA=http://127.0.0.1:3001 +APP_ORIGIN_AUTH=http://127.0.0.1:3010 +APP_ORIGIN_CONTROL=http://127.0.0.1:3003 +APP_ORIGIN_LTI=http://127.0.0.1:4000 +APP_ORIGIN_MANAGE=http://127.0.0.1:3002 +APP_ORIGIN_PWA=http://127.0.0.1:3001
5-5: Validate CORS origins vs newly introduced APP_ORIGIN_ for local dev.*CORS_ALLOWED_ORIGINS currently whitelists only :3002 locally, while APP_ORIGIN_{API,PWA} use :3000/:3001. If response-api receives browser requests from those apps, add them here to avoid CORS issues.
If needed, update like:
-CORS_ALLOWED_ORIGINS=http://localhost:3002,http://127.0.0.1:3002,https://pwa.klicker.com,https://assessment.klicker.com +CORS_ALLOWED_ORIGINS=http://localhost:3000,http://127.0.0.1:3000,http://localhost:3001,http://127.0.0.1:3001,http://localhost:3002,http://127.0.0.1:3002,https://pwa.klicker.com,https://assessment.klicker.comapps/hatchet-worker-general/.env.example (1)
6-13: Silence dotenv-linter: drop quotes and sort APP_ORIGIN_ keys*Values don’t require quotes and sorting reduces linter noise across env files.
-APP_ORIGIN_AUTH="http://127.0.0.1:3010" -APP_ORIGIN_API="http://127.0.0.1:3000" -APP_ORIGIN_LTI="http://127.0.0.1:4000" -APP_ORIGIN_PWA="http://127.0.0.1:3001" -APP_ORIGIN_MANAGE="http://127.0.0.1:3002" -APP_ORIGIN_CONTROL="http://127.0.0.1:3003" -APP_ORIGIN_ASSESSMENT_API="http://127.0.0.1:3000" -APP_ORIGIN_ASSESSMENT_PWA="http://127.0.0.1:3001" +APP_ORIGIN_API=http://127.0.0.1:3000 +APP_ORIGIN_ASSESSMENT_API=http://127.0.0.1:3000 +APP_ORIGIN_ASSESSMENT_PWA=http://127.0.0.1:3001 +APP_ORIGIN_AUTH=http://127.0.0.1:3010 +APP_ORIGIN_CONTROL=http://127.0.0.1:3003 +APP_ORIGIN_LTI=http://127.0.0.1:4000 +APP_ORIGIN_MANAGE=http://127.0.0.1:3002 +APP_ORIGIN_PWA=http://127.0.0.1:3001apps/frontend-control/.env.development (1)
8-23: Normalize ordering of APP_ORIGIN_ and NEXT_PUBLIC_ to satisfy dotenv-linter**Functionally fine; sorting keeps linters quiet and aids consistency.
-APP_ORIGIN_API=https://api.klicker.com -APP_ORIGIN_PWA=https://pwa.klicker.com -APP_ORIGIN_MANAGE=https://manage.klicker.com -APP_ORIGIN_CONTROL=https://control.klicker.com -APP_ORIGIN_LTI=https://lti.klicker.com -APP_ORIGIN_AUTH=https://auth.klicker.com -APP_ORIGIN_ASSESSMENT_API=https://assessment-api.klicker.com -APP_ORIGIN_ASSESSMENT_PWA=https://assessment.klicker.com +APP_ORIGIN_API=https://api.klicker.com +APP_ORIGIN_ASSESSMENT_API=https://assessment-api.klicker.com +APP_ORIGIN_ASSESSMENT_PWA=https://assessment.klicker.com +APP_ORIGIN_AUTH=https://auth.klicker.com +APP_ORIGIN_CONTROL=https://control.klicker.com +APP_ORIGIN_LTI=https://lti.klicker.com +APP_ORIGIN_MANAGE=https://manage.klicker.com +APP_ORIGIN_PWA=https://pwa.klicker.com -NEXT_PUBLIC_PWA_URL=$APP_ORIGIN_PWA -NEXT_PUBLIC_MANAGE_URL=$APP_ORIGIN_MANAGE -NEXT_PUBLIC_CONTROL_URL=$APP_ORIGIN_CONTROL -NEXT_PUBLIC_AUTH_URL=$APP_ORIGIN_AUTH -NEXT_PUBLIC_LTI_URL=$APP_ORIGIN_LTI -NEXT_PUBLIC_ASSESSMENT_URL=$APP_ORIGIN_ASSESSMENT_PWA -NEXT_PUBLIC_ASSESSMENT_API_URL=$APP_ORIGIN_ASSESSMENT_API +NEXT_PUBLIC_ASSESSMENT_API_URL=$APP_ORIGIN_ASSESSMENT_API +NEXT_PUBLIC_ASSESSMENT_URL=$APP_ORIGIN_ASSESSMENT_PWA +NEXT_PUBLIC_AUTH_URL=$APP_ORIGIN_AUTH +NEXT_PUBLIC_CONTROL_URL=$APP_ORIGIN_CONTROL +NEXT_PUBLIC_LTI_URL=$APP_ORIGIN_LTI +NEXT_PUBLIC_MANAGE_URL=$APP_ORIGIN_MANAGE +NEXT_PUBLIC_PWA_URL=$APP_ORIGIN_PWAapps/frontend-pwa/.env.test (2)
4-4: AI summary inconsistency: APP_SECRET is present hereThe summary says APP_SECRET was removed, but Line 4 defines it. Confirm intent; if redundant, drop it to avoid drift with other envs.
13-28: Sort APP_ORIGIN_ and NEXT_PUBLIC_ keys to reduce lint noise**No behavior change; aligns with other env files.
-APP_ORIGIN_API=http://127.0.0.1:3000 -APP_ORIGIN_PWA=http://127.0.0.1:3001 -APP_ORIGIN_MANAGE=http://127.0.0.1:3002 -APP_ORIGIN_CONTROL=http://127.0.0.1:3003 -APP_ORIGIN_LTI=http://127.0.0.1:4000 -APP_ORIGIN_AUTH=http://127.0.0.1:3010 -APP_ORIGIN_ASSESSMENT_API=http://127.0.0.1:3000 -APP_ORIGIN_ASSESSMENT_PWA=http://127.0.0.1:3001 +APP_ORIGIN_API=http://127.0.0.1:3000 +APP_ORIGIN_ASSESSMENT_API=http://127.0.0.1:3000 +APP_ORIGIN_ASSESSMENT_PWA=http://127.0.0.1:3001 +APP_ORIGIN_AUTH=http://127.0.0.1:3010 +APP_ORIGIN_CONTROL=http://127.0.0.1:3003 +APP_ORIGIN_LTI=http://127.0.0.1:4000 +APP_ORIGIN_MANAGE=http://127.0.0.1:3002 +APP_ORIGIN_PWA=http://127.0.0.1:3001 -NEXT_PUBLIC_PWA_URL=$APP_ORIGIN_PWA -NEXT_PUBLIC_MANAGE_URL=$APP_ORIGIN_MANAGE -NEXT_PUBLIC_CONTROL_URL=$APP_ORIGIN_CONTROL -NEXT_PUBLIC_AUTH_URL=$APP_ORIGIN_AUTH -NEXT_PUBLIC_LTI_URL=$APP_ORIGIN_LTI -NEXT_PUBLIC_ASSESSMENT_URL=$APP_ORIGIN_ASSESSMENT_PWA -NEXT_PUBLIC_ASSESSMENT_API_URL=$APP_ORIGIN_ASSESSMENT_API +NEXT_PUBLIC_ASSESSMENT_API_URL=$APP_ORIGIN_ASSESSMENT_API +NEXT_PUBLIC_ASSESSMENT_URL=$APP_ORIGIN_ASSESSMENT_PWA +NEXT_PUBLIC_AUTH_URL=$APP_ORIGIN_AUTH +NEXT_PUBLIC_CONTROL_URL=$APP_ORIGIN_CONTROL +NEXT_PUBLIC_LTI_URL=$APP_ORIGIN_LTI +NEXT_PUBLIC_MANAGE_URL=$APP_ORIGIN_MANAGE +NEXT_PUBLIC_PWA_URL=$APP_ORIGIN_PWAapps/hatchet-worker-response-processor/.env.example (1)
8-15: Make env entries linter-friendly: remove quotes and sortMinor tidy-up; matches the pattern used elsewhere.
-APP_ORIGIN_AUTH="http://127.0.0.1:3010" -APP_ORIGIN_API="http://127.0.0.1:3000" -APP_ORIGIN_LTI="http://127.0.0.1:4000" -APP_ORIGIN_PWA="http://127.0.0.1:3001" -APP_ORIGIN_MANAGE="http://127.0.0.1:3002" -APP_ORIGIN_CONTROL="http://127.0.0.1:3003" -APP_ORIGIN_ASSESSMENT_API="http://127.0.0.1:3000" -APP_ORIGIN_ASSESSMENT_PWA="http://127.0.0.1:3001" +APP_ORIGIN_API=http://127.0.0.1:3000 +APP_ORIGIN_ASSESSMENT_API=http://127.0.0.1:3000 +APP_ORIGIN_ASSESSMENT_PWA=http://127.0.0.1:3001 +APP_ORIGIN_AUTH=http://127.0.0.1:3010 +APP_ORIGIN_CONTROL=http://127.0.0.1:3003 +APP_ORIGIN_LTI=http://127.0.0.1:4000 +APP_ORIGIN_MANAGE=http://127.0.0.1:3002 +APP_ORIGIN_PWA=http://127.0.0.1:3001apps/auth/.env.development (1)
6-24: Good centralization; consider sorting keys to appease dotenv-linterThe move to APP_ORIGIN_* and deriving NEXTAUTH_URL/NEXT_PUBLIC_* from them is solid. Sorting avoids persistent linter warnings.
-APP_ORIGIN_API=https://api.klicker.com -APP_ORIGIN_PWA=https://pwa.klicker.com -APP_ORIGIN_MANAGE=https://manage.klicker.com -APP_ORIGIN_CONTROL=https://control.klicker.com -APP_ORIGIN_LTI=https://lti.klicker.com -APP_ORIGIN_AUTH=https://auth.klicker.com -APP_ORIGIN_ASSESSMENT_API=https://assessment-api.klicker.com -APP_ORIGIN_ASSESSMENT_PWA=https://assessment.klicker.com +APP_ORIGIN_API=https://api.klicker.com +APP_ORIGIN_ASSESSMENT_API=https://assessment-api.klicker.com +APP_ORIGIN_ASSESSMENT_PWA=https://assessment.klicker.com +APP_ORIGIN_AUTH=https://auth.klicker.com +APP_ORIGIN_CONTROL=https://control.klicker.com +APP_ORIGIN_LTI=https://lti.klicker.com +APP_ORIGIN_MANAGE=https://manage.klicker.com +APP_ORIGIN_PWA=https://pwa.klicker.com -NEXT_PUBLIC_PWA_URL=$APP_ORIGIN_PWA -NEXT_PUBLIC_MANAGE_URL=$APP_ORIGIN_MANAGE -NEXT_PUBLIC_CONTROL_URL=$APP_ORIGIN_CONTROL -NEXT_PUBLIC_AUTH_URL=$APP_ORIGIN_AUTH -NEXT_PUBLIC_LTI_URL=$APP_ORIGIN_LTI -NEXT_PUBLIC_ASSESSMENT_URL=$APP_ORIGIN_ASSESSMENT_PWA -NEXT_PUBLIC_ASSESSMENT_API_URL=$APP_ORIGIN_ASSESSMENT_API +NEXT_PUBLIC_ASSESSMENT_API_URL=$APP_ORIGIN_ASSESSMENT_API +NEXT_PUBLIC_ASSESSMENT_URL=$APP_ORIGIN_ASSESSMENT_PWA +NEXT_PUBLIC_AUTH_URL=$APP_ORIGIN_AUTH +NEXT_PUBLIC_CONTROL_URL=$APP_ORIGIN_CONTROL +NEXT_PUBLIC_LTI_URL=$APP_ORIGIN_LTI +NEXT_PUBLIC_MANAGE_URL=$APP_ORIGIN_MANAGE +NEXT_PUBLIC_PWA_URL=$APP_ORIGIN_PWAapps/backend-docker/.env.cypress (1)
16-24: Nice: canonicalize origins; minor lint tidy — quotes and sortKeep the helpful comments. For the vars, removing quotes and sorting keeps linters green across envs.
-APP_ORIGIN_AUTH="http://127.0.0.1:3010" -APP_ORIGIN_API="http://127.0.0.1:3000" -APP_ORIGIN_LTI="http://127.0.0.1:4000" -APP_ORIGIN_PWA="http://127.0.0.1:3001" -APP_ORIGIN_MANAGE="http://127.0.0.1:3002" -APP_ORIGIN_CONTROL="http://127.0.0.1:3003" -APP_ORIGIN_ASSESSMENT_API="http://127.0.0.1:3000" -APP_ORIGIN_ASSESSMENT_PWA="http://127.0.0.1:3001" +APP_ORIGIN_API=http://127.0.0.1:3000 +APP_ORIGIN_ASSESSMENT_API=http://127.0.0.1:3000 +APP_ORIGIN_ASSESSMENT_PWA=http://127.0.0.1:3001 +APP_ORIGIN_AUTH=http://127.0.0.1:3010 +APP_ORIGIN_CONTROL=http://127.0.0.1:3003 +APP_ORIGIN_LTI=http://127.0.0.1:4000 +APP_ORIGIN_MANAGE=http://127.0.0.1:3002 +APP_ORIGIN_PWA=http://127.0.0.1:3001apps/auth/.env.qa (1)
6-24: QA env: centralization looks good; sort keys to avoid linter warningsSame rationale as dev env.
-APP_ORIGIN_API=https://api.klicker-qa.bf-app.ch -APP_ORIGIN_PWA=https://pwa.klicker-qa.bf-app.ch -APP_ORIGIN_MANAGE=https://manage.klicker-qa.bf-app.ch -APP_ORIGIN_CONTROL=https://control.klicker-qa.bf-app.ch -APP_ORIGIN_LTI=https://lti.klicker-qa.bf-app.ch -APP_ORIGIN_AUTH=https://auth.klicker-qa.bf-app.ch -APP_ORIGIN_ASSESSMENT_API=https://assessment-api.klicker-qa.bf-app.ch -APP_ORIGIN_ASSESSMENT_PWA=https://assessment.klicker-qa.bf-app.ch +APP_ORIGIN_API=https://api.klicker-qa.bf-app.ch +APP_ORIGIN_ASSESSMENT_API=https://assessment-api.klicker-qa.bf-app.ch +APP_ORIGIN_ASSESSMENT_PWA=https://assessment.klicker-qa.bf-app.ch +APP_ORIGIN_AUTH=https://auth.klicker-qa.bf-app.ch +APP_ORIGIN_CONTROL=https://control.klicker-qa.bf-app.ch +APP_ORIGIN_LTI=https://lti.klicker-qa.bf-app.ch +APP_ORIGIN_MANAGE=https://manage.klicker-qa.bf-app.ch +APP_ORIGIN_PWA=https://pwa.klicker-qa.bf-app.ch -NEXT_PUBLIC_PWA_URL=$APP_ORIGIN_PWA -NEXT_PUBLIC_MANAGE_URL=$APP_ORIGIN_MANAGE -NEXT_PUBLIC_CONTROL_URL=$APP_ORIGIN_CONTROL -NEXT_PUBLIC_AUTH_URL=$APP_ORIGIN_AUTH -NEXT_PUBLIC_LTI_URL=$APP_ORIGIN_LTI -NEXT_PUBLIC_ASSESSMENT_URL=$APP_ORIGIN_ASSESSMENT_PWA -NEXT_PUBLIC_ASSESSMENT_API_URL=$APP_ORIGIN_ASSESSMENT_API +NEXT_PUBLIC_ASSESSMENT_API_URL=$APP_ORIGIN_ASSESSMENT_API +NEXT_PUBLIC_ASSESSMENT_URL=$APP_ORIGIN_ASSESSMENT_PWA +NEXT_PUBLIC_AUTH_URL=$APP_ORIGIN_AUTH +NEXT_PUBLIC_CONTROL_URL=$APP_ORIGIN_CONTROL +NEXT_PUBLIC_LTI_URL=$APP_ORIGIN_LTI +NEXT_PUBLIC_MANAGE_URL=$APP_ORIGIN_MANAGE +NEXT_PUBLIC_PWA_URL=$APP_ORIGIN_PWAapps/frontend-manage/.env.development (1)
8-23: LGTM on centralizing origins for dev.Minor: consider alphabetic ordering (APP_ORIGIN_* then NEXT_PUBLIC_*) for consistency and to satisfy dotenv-linter.
apps/frontend-control/.env.test (1)
10-25: Test env origins: consistent and clear.No blockers. Optional key ordering to satisfy dotenv-linter.
apps/frontend-manage/.env.test (1)
8-23: Test env: centralized origins look good.Only nit: key ordering for dotenv-linter.
apps/backend-docker/.env.example (1)
14-24: Nit: remove quotes and sort APP_ORIGIN_ in apps/backend-docker/.env.example; document intentional ASSESSMENT_ duplicates**
- Keep these in sync with Helm/global ConfigMap (deploy/charts/klicker-uzh-v2/templates/cm-global.yaml).
- Apply the diff below to satisfy dotenv-linter (no quotes, sorted keys) and add a comment clarifying ASSESSMENT_API = API and ASSESSMENT_PWA = PWA for local dev.
-# Canonical app origins (prefer these; issuers fall back to them) -APP_ORIGIN_AUTH="http://127.0.0.1:3010" -APP_ORIGIN_API="http://127.0.0.1:3000" -APP_ORIGIN_LTI="http://127.0.0.1:4000" -APP_ORIGIN_PWA="http://127.0.0.1:3001" -APP_ORIGIN_MANAGE="http://127.0.0.1:3002" -APP_ORIGIN_CONTROL="http://127.0.0.1:3003" -APP_ORIGIN_ASSESSMENT_API="http://127.0.0.1:3000" -APP_ORIGIN_ASSESSMENT_PWA="http://127.0.0.1:3001" +# Canonical app origins (prefer these; issuers fall back to them) +# Note: In local dev, ASSESSMENT_API = API and ASSESSMENT_PWA = PWA on purpose. +APP_ORIGIN_API=http://127.0.0.1:3000 +APP_ORIGIN_ASSESSMENT_API=http://127.0.0.1:3000 +APP_ORIGIN_ASSESSMENT_PWA=http://127.0.0.1:3001 +APP_ORIGIN_AUTH=http://127.0.0.1:3010 +APP_ORIGIN_CONTROL=http://127.0.0.1:3003 +APP_ORIGIN_LTI=http://127.0.0.1:4000 +APP_ORIGIN_MANAGE=http://127.0.0.1:3002 +APP_ORIGIN_PWA=http://127.0.0.1:3001apps/frontend-manage/.env.qa (1)
8-23: Origins centralization looks good; please sort keys to satisfy dotenv-linter.Implementation is consistent and aligns NEXT_PUBLIC_* to APP_ORIGIN_*. To quiet the lint warnings, alphabetize the blocks.
-APP_ORIGIN_API=https://api.klicker-qa.bf-app.ch -APP_ORIGIN_PWA=https://pwa.klicker-qa.bf-app.ch -APP_ORIGIN_MANAGE=https://manage.klicker-qa.bf-app.ch -APP_ORIGIN_CONTROL=https://control.klicker-qa.bf-app.ch -APP_ORIGIN_LTI=https://lti.klicker-qa.bf-app.ch -APP_ORIGIN_AUTH=https://auth.klicker-qa.bf-app.ch -APP_ORIGIN_ASSESSMENT_API=https://assessment-api.klicker-qa.bf-app.ch -APP_ORIGIN_ASSESSMENT_PWA=https://assessment.klicker-qa.bf-app.ch +APP_ORIGIN_API=https://api.klicker-qa.bf-app.ch +APP_ORIGIN_ASSESSMENT_API=https://assessment-api.klicker-qa.bf-app.ch +APP_ORIGIN_ASSESSMENT_PWA=https://assessment.klicker-qa.bf-app.ch +APP_ORIGIN_AUTH=https://auth.klicker-qa.bf-app.ch +APP_ORIGIN_CONTROL=https://control.klicker-qa.bf-app.ch +APP_ORIGIN_LTI=https://lti.klicker-qa.bf-app.ch +APP_ORIGIN_MANAGE=https://manage.klicker-qa.bf-app.ch +APP_ORIGIN_PWA=https://pwa.klicker-qa.bf-app.ch -NEXT_PUBLIC_PWA_URL=$APP_ORIGIN_PWA -NEXT_PUBLIC_MANAGE_URL=$APP_ORIGIN_MANAGE -NEXT_PUBLIC_CONTROL_URL=$APP_ORIGIN_CONTROL -NEXT_PUBLIC_AUTH_URL=$APP_ORIGIN_AUTH -NEXT_PUBLIC_LTI_URL=$APP_ORIGIN_LTI -NEXT_PUBLIC_ASSESSMENT_URL=$APP_ORIGIN_ASSESSMENT_PWA -NEXT_PUBLIC_ASSESSMENT_API_URL=$APP_ORIGIN_ASSESSMENT_API +NEXT_PUBLIC_ASSESSMENT_API_URL=$APP_ORIGIN_ASSESSMENT_API +NEXT_PUBLIC_ASSESSMENT_URL=$APP_ORIGIN_ASSESSMENT_PWA +NEXT_PUBLIC_AUTH_URL=$APP_ORIGIN_AUTH +NEXT_PUBLIC_CONTROL_URL=$APP_ORIGIN_CONTROL +NEXT_PUBLIC_LTI_URL=$APP_ORIGIN_LTI +NEXT_PUBLIC_MANAGE_URL=$APP_ORIGIN_MANAGE +NEXT_PUBLIC_PWA_URL=$APP_ORIGIN_PWAapps/frontend-pwa/.env.production (1)
11-26: Consistent origin mapping; fix ordering to appease dotenv-linter.Values look correct for prod; only ordering nits remain.
-APP_ORIGIN_API=https://api.klicker.uzh.ch -APP_ORIGIN_PWA=https://pwa.klicker.uzh.ch -APP_ORIGIN_MANAGE=https://manage.klicker.uzh.ch -APP_ORIGIN_CONTROL=https://control.klicker.uzh.ch -APP_ORIGIN_LTI=https://lti.klicker.uzh.ch -APP_ORIGIN_AUTH=https://auth.klicker.uzh.ch -APP_ORIGIN_ASSESSMENT_API=https://assessment-api.klicker.uzh.ch -APP_ORIGIN_ASSESSMENT_PWA=https://assessment.klicker.uzh.ch +APP_ORIGIN_API=https://api.klicker.uzh.ch +APP_ORIGIN_ASSESSMENT_API=https://assessment-api.klicker.uzh.ch +APP_ORIGIN_ASSESSMENT_PWA=https://assessment.klicker.uzh.ch +APP_ORIGIN_AUTH=https://auth.klicker.uzh.ch +APP_ORIGIN_CONTROL=https://control.klicker.uzh.ch +APP_ORIGIN_LTI=https://lti.klicker.uzh.ch +APP_ORIGIN_MANAGE=https://manage.klicker.uzh.ch +APP_ORIGIN_PWA=https://pwa.klicker.uzh.ch -NEXT_PUBLIC_PWA_URL=$APP_ORIGIN_PWA -NEXT_PUBLIC_MANAGE_URL=$APP_ORIGIN_MANAGE -NEXT_PUBLIC_CONTROL_URL=$APP_ORIGIN_CONTROL -NEXT_PUBLIC_AUTH_URL=$APP_ORIGIN_AUTH -NEXT_PUBLIC_LTI_URL=$APP_ORIGIN_LTI -NEXT_PUBLIC_ASSESSMENT_URL=$APP_ORIGIN_ASSESSMENT_PWA -NEXT_PUBLIC_ASSESSMENT_API_URL=$APP_ORIGIN_ASSESSMENT_API +NEXT_PUBLIC_ASSESSMENT_API_URL=$APP_ORIGIN_ASSESSMENT_API +NEXT_PUBLIC_ASSESSMENT_URL=$APP_ORIGIN_ASSESSMENT_PWA +NEXT_PUBLIC_AUTH_URL=$APP_ORIGIN_AUTH +NEXT_PUBLIC_CONTROL_URL=$APP_ORIGIN_CONTROL +NEXT_PUBLIC_LTI_URL=$APP_ORIGIN_LTI +NEXT_PUBLIC_MANAGE_URL=$APP_ORIGIN_MANAGE +NEXT_PUBLIC_PWA_URL=$APP_ORIGIN_PWAapps/frontend-pwa/.env.assessment.qa (1)
11-26: QA assessment origins: OK; alphabetize for linter.Same nit as other envs.
-APP_ORIGIN_API=https://api.klicker-qa.bf-app.ch -APP_ORIGIN_PWA=https://pwa.klicker-qa.bf-app.ch -APP_ORIGIN_MANAGE=https://manage.klicker-qa.bf-app.ch -APP_ORIGIN_CONTROL=https://control.klicker-qa.bf-app.ch -APP_ORIGIN_LTI=https://lti.klicker-qa.bf-app.ch -APP_ORIGIN_AUTH=https://auth.klicker-qa.bf-app.ch -APP_ORIGIN_ASSESSMENT_API=https://assessment-api.klicker-qa.bf-app.ch -APP_ORIGIN_ASSESSMENT_PWA=https://assessment.klicker-qa.bf-app.ch +APP_ORIGIN_API=https://api.klicker-qa.bf-app.ch +APP_ORIGIN_ASSESSMENT_API=https://assessment-api.klicker-qa.bf-app.ch +APP_ORIGIN_ASSESSMENT_PWA=https://assessment.klicker-qa.bf-app.ch +APP_ORIGIN_AUTH=https://auth.klicker-qa.bf-app.ch +APP_ORIGIN_CONTROL=https://control.klicker-qa.bf-app.ch +APP_ORIGIN_LTI=https://lti.klicker-qa.bf-app.ch +APP_ORIGIN_MANAGE=https://manage.klicker-qa.bf-app.ch +APP_ORIGIN_PWA=https://pwa.klicker-qa.bf-app.ch -NEXT_PUBLIC_PWA_URL=$APP_ORIGIN_PWA -NEXT_PUBLIC_MANAGE_URL=$APP_ORIGIN_MANAGE -NEXT_PUBLIC_CONTROL_URL=$APP_ORIGIN_CONTROL -NEXT_PUBLIC_AUTH_URL=$APP_ORIGIN_AUTH -NEXT_PUBLIC_LTI_URL=$APP_ORIGIN_LTI -NEXT_PUBLIC_ASSESSMENT_URL=$APP_ORIGIN_ASSESSMENT_PWA -NEXT_PUBLIC_ASSESSMENT_API_URL=$APP_ORIGIN_ASSESSMENT_API +NEXT_PUBLIC_ASSESSMENT_API_URL=$APP_ORIGIN_ASSESSMENT_API +NEXT_PUBLIC_ASSESSMENT_URL=$APP_ORIGIN_ASSESSMENT_PWA +NEXT_PUBLIC_AUTH_URL=$APP_ORIGIN_AUTH +NEXT_PUBLIC_CONTROL_URL=$APP_ORIGIN_CONTROL +NEXT_PUBLIC_LTI_URL=$APP_ORIGIN_LTI +NEXT_PUBLIC_MANAGE_URL=$APP_ORIGIN_MANAGE +NEXT_PUBLIC_PWA_URL=$APP_ORIGIN_PWAapps/frontend-pwa/.env.assessment (1)
11-26: Assessment prod origins: fine; reorder for linter.-APP_ORIGIN_API=https://backend-sls.klicker.uzh.ch -APP_ORIGIN_PWA=https://pwa.klicker.uzh.ch -APP_ORIGIN_MANAGE=https://manage.klicker.uzh.ch -APP_ORIGIN_CONTROL=https://control.klicker.uzh.ch -APP_ORIGIN_LTI=https://lti.klicker.uzh.ch -APP_ORIGIN_AUTH=https://auth.klicker.uzh.ch -APP_ORIGIN_ASSESSMENT_API=https://assessment-api.klicker.uzh.ch -APP_ORIGIN_ASSESSMENT_PWA=https://assessment.klicker.uzh.ch +APP_ORIGIN_API=https://backend-sls.klicker.uzh.ch +APP_ORIGIN_ASSESSMENT_API=https://assessment-api.klicker.uzh.ch +APP_ORIGIN_ASSESSMENT_PWA=https://assessment.klicker.uzh.ch +APP_ORIGIN_AUTH=https://auth.klicker.uzh.ch +APP_ORIGIN_CONTROL=https://control.klicker.uzh.ch +APP_ORIGIN_LTI=https://lti.klicker.uzh.ch +APP_ORIGIN_MANAGE=https://manage.klicker.uzh.ch +APP_ORIGIN_PWA=https://pwa.klicker.uzh.ch -NEXT_PUBLIC_PWA_URL=$APP_ORIGIN_PWA -NEXT_PUBLIC_MANAGE_URL=$APP_ORIGIN_MANAGE -NEXT_PUBLIC_CONTROL_URL=$APP_ORIGIN_CONTROL -NEXT_PUBLIC_AUTH_URL=$APP_ORIGIN_AUTH -NEXT_PUBLIC_LTI_URL=$APP_ORIGIN_LTI -NEXT_PUBLIC_ASSESSMENT_URL=$APP_ORIGIN_ASSESSMENT_PWA -NEXT_PUBLIC_ASSESSMENT_API_URL=$APP_ORIGIN_ASSESSMENT_API +NEXT_PUBLIC_ASSESSMENT_API_URL=$APP_ORIGIN_ASSESSMENT_API +NEXT_PUBLIC_ASSESSMENT_URL=$APP_ORIGIN_ASSESSMENT_PWA +NEXT_PUBLIC_AUTH_URL=$APP_ORIGIN_AUTH +NEXT_PUBLIC_CONTROL_URL=$APP_ORIGIN_CONTROL +NEXT_PUBLIC_LTI_URL=$APP_ORIGIN_LTI +NEXT_PUBLIC_MANAGE_URL=$APP_ORIGIN_MANAGE +NEXT_PUBLIC_PWA_URL=$APP_ORIGIN_PWAapps/frontend-pwa/.env.development (1)
14-29: Dev origins mapping: LGTM; sort keys for linter consistency.-APP_ORIGIN_API=https://api.klicker.com -APP_ORIGIN_PWA=https://pwa.klicker.com -APP_ORIGIN_MANAGE=https://manage.klicker.com -APP_ORIGIN_CONTROL=https://control.klicker.com -APP_ORIGIN_LTI=https://lti.klicker.com -APP_ORIGIN_AUTH=https://auth.klicker.com -APP_ORIGIN_ASSESSMENT_API=https://assessment-api.klicker.com -APP_ORIGIN_ASSESSMENT_PWA=https://assessment.klicker.com +APP_ORIGIN_API=https://api.klicker.com +APP_ORIGIN_ASSESSMENT_API=https://assessment-api.klicker.com +APP_ORIGIN_ASSESSMENT_PWA=https://assessment.klicker.com +APP_ORIGIN_AUTH=https://auth.klicker.com +APP_ORIGIN_CONTROL=https://control.klicker.com +APP_ORIGIN_LTI=https://lti.klicker.com +APP_ORIGIN_MANAGE=https://manage.klicker.com +APP_ORIGIN_PWA=https://pwa.klicker.com -NEXT_PUBLIC_PWA_URL=$APP_ORIGIN_PWA -NEXT_PUBLIC_MANAGE_URL=$APP_ORIGIN_MANAGE -NEXT_PUBLIC_CONTROL_URL=$APP_ORIGIN_CONTROL -NEXT_PUBLIC_AUTH_URL=$APP_ORIGIN_AUTH -NEXT_PUBLIC_LTI_URL=$APP_ORIGIN_LTI -NEXT_PUBLIC_ASSESSMENT_URL=$APP_ORIGIN_ASSESSMENT_PWA -NEXT_PUBLIC_ASSESSMENT_API_URL=$APP_ORIGIN_ASSESSMENT_API +NEXT_PUBLIC_ASSESSMENT_API_URL=$APP_ORIGIN_ASSESSMENT_API +NEXT_PUBLIC_ASSESSMENT_URL=$APP_ORIGIN_ASSESSMENT_PWA +NEXT_PUBLIC_AUTH_URL=$APP_ORIGIN_AUTH +NEXT_PUBLIC_CONTROL_URL=$APP_ORIGIN_CONTROL +NEXT_PUBLIC_LTI_URL=$APP_ORIGIN_LTI +NEXT_PUBLIC_MANAGE_URL=$APP_ORIGIN_MANAGE +NEXT_PUBLIC_PWA_URL=$APP_ORIGIN_PWA
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
apps/frontend-pwa/.env.production (2)
11-18: Satisfy dotenv-linter: sort APP_ORIGIN_ keys alphabetically.*Purely cosmetic; keeps CI clean.
-APP_ORIGIN_API=https://backend-sls.klicker.uzh.ch -APP_ORIGIN_PWA=https://pwa.klicker.uzh.ch -APP_ORIGIN_MANAGE=https://manage.klicker.uzh.ch -APP_ORIGIN_CONTROL=https://control.klicker.uzh.ch -APP_ORIGIN_LTI=https://lti.klicker.uzh.ch -APP_ORIGIN_AUTH=https://auth.klicker.uzh.ch -APP_ORIGIN_ASSESSMENT_API=https://assessment-api.klicker.uzh.ch -APP_ORIGIN_ASSESSMENT_PWA=https://assessment.klicker.uzh.ch +APP_ORIGIN_API=https://backend-sls.klicker.uzh.ch +APP_ORIGIN_ASSESSMENT_API=https://assessment-api.klicker.uzh.ch +APP_ORIGIN_ASSESSMENT_PWA=https://assessment.klicker.uzh.ch +APP_ORIGIN_AUTH=https://auth.klicker.uzh.ch +APP_ORIGIN_CONTROL=https://control.klicker.uzh.ch +APP_ORIGIN_LTI=https://lti.klicker.uzh.ch +APP_ORIGIN_MANAGE=https://manage.klicker.uzh.ch +APP_ORIGIN_PWA=https://pwa.klicker.uzh.ch
20-26: Satisfy dotenv-linter: sort NEXT_PUBLIC_ URLs alphabetically.*Also cosmetic; matches linter suggestions.
-NEXT_PUBLIC_PWA_URL=$APP_ORIGIN_PWA -NEXT_PUBLIC_MANAGE_URL=$APP_ORIGIN_MANAGE -NEXT_PUBLIC_CONTROL_URL=$APP_ORIGIN_CONTROL -NEXT_PUBLIC_AUTH_URL=$APP_ORIGIN_AUTH -NEXT_PUBLIC_LTI_URL=$APP_ORIGIN_LTI -NEXT_PUBLIC_ASSESSMENT_URL=$APP_ORIGIN_ASSESSMENT_PWA -NEXT_PUBLIC_ASSESSMENT_API_URL=$APP_ORIGIN_ASSESSMENT_API +NEXT_PUBLIC_ASSESSMENT_API_URL=$APP_ORIGIN_ASSESSMENT_API +NEXT_PUBLIC_ASSESSMENT_URL=$APP_ORIGIN_ASSESSMENT_PWA +NEXT_PUBLIC_AUTH_URL=$APP_ORIGIN_AUTH +NEXT_PUBLIC_CONTROL_URL=$APP_ORIGIN_CONTROL +NEXT_PUBLIC_LTI_URL=$APP_ORIGIN_LTI +NEXT_PUBLIC_MANAGE_URL=$APP_ORIGIN_MANAGE +NEXT_PUBLIC_PWA_URL=$APP_ORIGIN_PWA
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
apps/frontend-pwa/.env.production(1 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-06-23T12:33:32.937Z
Learnt from: CR
PR: uzh-bf/klicker-uzh#0
File: packages/next-config/CLAUDE.md:0-0
Timestamp: 2025-06-23T12:33:32.937Z
Learning: The klicker-uzh/next-config package provides standardized Next.js configuration functions (getNextBaseConfig and getNextPWAConfig) to ensure consistency and reduce duplication across all frontend applications in the KlickerUZH ecosystem.
Applied to files:
apps/frontend-pwa/.env.production
🪛 dotenv-linter (3.3.0)
apps/frontend-pwa/.env.production
[warning] 13-13: [UnorderedKey] The APP_ORIGIN_MANAGE key should go before the APP_ORIGIN_PWA key
(UnorderedKey)
[warning] 14-14: [UnorderedKey] The APP_ORIGIN_CONTROL key should go before the APP_ORIGIN_MANAGE key
(UnorderedKey)
[warning] 15-15: [UnorderedKey] The APP_ORIGIN_LTI key should go before the APP_ORIGIN_MANAGE key
(UnorderedKey)
[warning] 16-16: [UnorderedKey] The APP_ORIGIN_AUTH key should go before the APP_ORIGIN_CONTROL key
(UnorderedKey)
[warning] 17-17: [UnorderedKey] The APP_ORIGIN_ASSESSMENT_API key should go before the APP_ORIGIN_AUTH key
(UnorderedKey)
[warning] 18-18: [UnorderedKey] The APP_ORIGIN_ASSESSMENT_PWA key should go before the APP_ORIGIN_AUTH key
(UnorderedKey)
[warning] 21-21: [UnorderedKey] The NEXT_PUBLIC_MANAGE_URL key should go before the NEXT_PUBLIC_PWA_URL key
(UnorderedKey)
[warning] 22-22: [UnorderedKey] The NEXT_PUBLIC_CONTROL_URL key should go before the NEXT_PUBLIC_MANAGE_URL key
(UnorderedKey)
[warning] 23-23: [UnorderedKey] The NEXT_PUBLIC_AUTH_URL key should go before the NEXT_PUBLIC_CONTROL_URL key
(UnorderedKey)
[warning] 24-24: [UnorderedKey] The NEXT_PUBLIC_LTI_URL key should go before the NEXT_PUBLIC_MANAGE_URL key
(UnorderedKey)
[warning] 25-25: [UnorderedKey] The NEXT_PUBLIC_ASSESSMENT_URL key should go before the NEXT_PUBLIC_AUTH_URL key
(UnorderedKey)
[warning] 26-26: [UnorderedKey] The NEXT_PUBLIC_ASSESSMENT_API_URL key should go before the NEXT_PUBLIC_ASSESSMENT_URL key
(UnorderedKey)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (10)
- GitHub Check: cypress-run-cloud
- GitHub Check: test
- GitHub Check: build
- GitHub Check: SonarCloud
- GitHub Check: Analyze (javascript)
- GitHub Check: build
- GitHub Check: format
- GitHub Check: build
- GitHub Check: build
- GitHub Check: check
🔇 Additional comments (3)
apps/frontend-pwa/.env.production (3)
20-26: Confirm dotenv expansion and avoid exposing SSR endpointsFile: apps/frontend-pwa/.env.production (lines 20–26). Automated ripgrep returned "No files were searched" so I couldn't verify — manually confirm:
- Ensure dotenv expansion is enabled for frontend-pwa so $APP_ORIGIN_* is expanded at build time (e.g., dotenv-expand or equivalent in your Next/ build pipeline).
- Ensure server-only endpoints remain server-only: locate any API_URL_SSR and remove any NEXT_PUBLIC_API_URL_SSR usages (keep API_URL_SSR out of client-exposed NEXT_PUBLIC_ vars).
Run these locally and share results if you want me to re-check:
# search for server-only and client-exposed SSR envs rg -n --hidden --glob '!node_modules/**' '\b(API_URL_SSR|NEXT_PUBLIC_API_URL_SSR)\b' -C2 || git grep -n 'API_URL_SSR\|NEXT_PUBLIC_API_URL_SSR' || true # check for dotenv expansion usage in the repo rg -n --hidden --glob '!node_modules/**' 'dotenv-expand|@next/env' -C2 || git grep -n 'dotenv-expand\|@next/env' || true # if rg reports "No files were searched", run rg with --debug to diagnose or use the git grep fallbacks above
24-26: Update CSP/CORS allowlists to include the new public origins (LTI & Assessment)Add NEXT_PUBLIC_LTI_URL, NEXT_PUBLIC_ASSESSMENT_URL and NEXT_PUBLIC_ASSESSMENT_API_URL to connect-src and any GraphQL/WebSocket endpoints; update frame-ancestors (if embedding) and the Assessment API CORS allowlist to prevent runtime blocks.
Check/update these places:
- apps/frontend-pwa/next.config.js (headers) and any server/middleware that calls setHeader(...)
- apps/frontend-pwa/pages/_document.js or public/_document.js (meta CSP)
- Backend API/GraphQL/WebSocket CORS config (Assessment API)
- Reverse proxy / infra configs (nginx, CDN, API gateway) and platform settings (Vercel/Netlify)
Repo search returned no CSP/CORS rules in the repo — verify platform/infra-level CSP/CORS and confirm after applying changes.
11-18: Confirm APP_ORIGIN_ are used as JWT 'iss' and provisioned consistently.*
apps/frontend-pwa/.env.production declares the canonical origins; ensure these exact values are used as the JWT issuer ("iss") and are provisioned via Helm/ConfigMaps for prod and stage to avoid issuer drift. Automated repo search returned no 'issuer'/'iss' references — verify issuer configuration and ConfigMaps/Helm values.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
packages/prisma/package.json (1)
58-60: Studio via wrapper: OK.Nice consistency. Consider whether Studio should default to dev only to avoid accidental prod connections. If intentional, keep.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
apps/frontend-control/src/lib/apollo.ts(1 hunks)apps/frontend-manage/src/lib/apollo.ts(1 hunks)apps/frontend-pwa/src/lib/apollo.ts(1 hunks)apps/frontend-pwa/src/pages/createAccount.tsx(2 hunks)packages/prisma/package.json(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (4)
- apps/frontend-manage/src/lib/apollo.ts
- apps/frontend-pwa/src/lib/apollo.ts
- apps/frontend-pwa/src/pages/createAccount.tsx
- apps/frontend-control/src/lib/apollo.ts
🧰 Additional context used
🪛 Checkov (3.2.334)
packages/prisma/package.json
[medium] 41-42: Basic Auth Credentials
(CKV_SECRET_4)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (14)
- GitHub Check: cypress-run-cloud
- GitHub Check: build
- GitHub Check: build
- GitHub Check: SonarCloud
- GitHub Check: build
- GitHub Check: build
- GitHub Check: test
- GitHub Check: format
- GitHub Check: Analyze (javascript)
- GitHub Check: build
- GitHub Check: build
- GitHub Check: test
- GitHub Check: check
- GitHub Check: build
🔇 Additional comments (3)
packages/prisma/package.json (3)
42-45: Looks consistent with the new pattern.Dev migrate and push routed through the wrapper; QA push maps to CONFIG=stg. No issues spotted.
37-39: Confirm wrapper exists & is executable — verify CONFIG handlingutil/_run_with_doppler.sh is present at util/_run_with_doppler.sh and is executable (header shown). Confirm the script reads/handles CONFIG=dev|stg|prd (look for $CONFIG, "case $CONFIG", or explicit parsing).
47-52: Bug: -f may be consumed by pnpm; forward it or call Prisma directly.pnpm's docs say args after the script are forwarded, but flag parsing has been historically inconsistent; Prisma's CLI accepts -f/--force. Use one of the fixes below to guarantee the flag reaches Prisma. (pnpm.io)
Apply one of these:
- "prisma:reset:yes": "CONFIG=dev ../../util/_run_with_doppler.sh pnpm run prisma:reset:raw -f", + "prisma:reset:yes": "CONFIG=dev ../../util/_run_with_doppler.sh pnpm run prisma:reset:raw -- -f",or avoid pnpm:
- "prisma:reset:yes": "CONFIG=dev ../../util/_run_with_doppler.sh pnpm run prisma:reset:raw -f", + "prisma:reset:yes": "CONFIG=dev ../../util/_run_with_doppler.sh prisma migrate reset -f --skip-seed",Also confirm prisma:resetCypress intentionally bypasses Doppler (packages/prisma/package.json) and that DATABASE_URL is set where it's run (CI/workflows).
| "prisma:deploy:prod": "CONFIG=prd ../../util/_run_with_doppler.sh pnpm run prisma:deploy:raw", | ||
| "prisma:deploy:qa": "CONFIG=stg ../../util/_run_with_doppler.sh pnpm run prisma:deploy:raw", | ||
| "prisma:deploy:raw": "prisma migrate deploy", | ||
| "prisma:diff": "doppler run --config dev --command 'prisma migrate diff --from-url \"$PROD_DATABASE_URL\" --to-migrations src/prisma/migrations --shadow-database-url \"postgres://klicker-prod:klicker@localhost:5432/shadow\"'", |
There was a problem hiding this comment.
Leak of hardcoded DB credentials; also contradicts PR summary (“prisma:diff removed”).
The prisma:diff script still exists and embeds basic auth creds in a URL. Remove it or migrate to the wrapper with secrets from Doppler.
Apply this diff to remove the script:
- "prisma:diff": "doppler run --config dev --command 'prisma migrate diff --from-url \"$PROD_DATABASE_URL\" --to-migrations src/prisma/migrations --shadow-database-url \"postgres://klicker-prod:klicker@localhost:5432/shadow\"'",If you must keep it, use the wrapper and secret envs (no inline creds):
- "prisma:diff": "doppler run --config dev --command 'prisma migrate diff --from-url \"$PROD_DATABASE_URL\" --to-migrations src/prisma/migrations --shadow-database-url \"postgres://klicker-prod:klicker@localhost:5432/shadow\"'",
+ "prisma:diff": "CONFIG=dev ../../util/_run_with_doppler.sh sh -lc 'prisma migrate diff --from-url \"$PROD_DATABASE_URL\" --to-migrations src/prisma/migrations --shadow-database-url \"$SHADOW_DATABASE_URL\"'"📝 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.
| "prisma:diff": "doppler run --config dev --command 'prisma migrate diff --from-url \"$PROD_DATABASE_URL\" --to-migrations src/prisma/migrations --shadow-database-url \"postgres://klicker-prod:klicker@localhost:5432/shadow\"'", | |
| "prisma:diff": "CONFIG=dev ../../util/_run_with_doppler.sh sh -lc 'prisma migrate diff --from-url \"$PROD_DATABASE_URL\" --to-migrations src/prisma/migrations --shadow-database-url \"$SHADOW_DATABASE_URL\"'" |
🧰 Tools
🪛 Checkov (3.2.334)
[medium] 41-42: Basic Auth Credentials
(CKV_SECRET_4)
🤖 Prompt for AI Agents
In packages/prisma/package.json around line 41, the "prisma:diff" npm script
contains hardcoded DB credentials and contradicts the PR summary stating it was
removed; either delete this script entry from package.json or replace it to use
the Doppler wrapper and environment variables (e.g., doppler run --config dev
--command 'prisma migrate diff --from-url "$PROD_DATABASE_URL" --to-migrations
src/prisma/migrations --shadow-database-url "$SHADOW_DATABASE_URL"') so no
credentials are inlined, and update the PR summary to reflect the actual change
(removed or preserved with secrets).
klicker-uzh
|
||||||||||||||||||||||||||||||
| Project |
klicker-uzh
|
| Branch Review |
jwt-issuer-v2
|
| Run status |
|
| Run duration | 19m 04s |
| Commit |
|
| Committer | Roland Schläfli |
| View all properties for this run ↗︎ | |
| Test results | |
|---|---|
Failures
|
35
|
Flaky
|
1
|
Pending
|
0
|
Skipped
|
0
|
Passing
|
722
|
Upgrade your plan to view test results. | |
| View all changes introduced in this branch ↗︎ | |
Tests for review
…into jwt-issuer-v2 # Conflicts: # .github/workflows/cypress-testing.yml # .github/workflows/test-graphql.yml # apps/auth/.env.development # apps/auth/.env.production # apps/auth/.env.qa # apps/auth/.env.test # apps/auth/src/pages/api/auth/[...nextauth].ts # apps/backend-docker/.env.cypress # apps/backend-docker/.env.example
|
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (3)
packages/prisma/package.json (1)
41-41: Remove hardcoded DB credentials from prisma:diff (secrets leak).This still embeds basic-auth credentials and contradicts the intent to eliminate direct Doppler invocations. Either delete the script or switch to the wrapper with env-provided secrets. Checkov also flags this (CKV_SECRET_4).
Apply this diff to remove it:
- "prisma:diff": "doppler run --config dev --command 'prisma migrate diff --from-url \"$PROD_DATABASE_URL\" --to-migrations src/prisma/migrations --shadow-database-url \"postgres://klicker-prod:klicker@localhost:5432/shadow\"'",Optional (if you must keep it), use the wrapper and secret envs:
- "prisma:diff": "doppler run --config dev --command 'prisma migrate diff --from-url \"$PROD_DATABASE_URL\" --to-migrations src/prisma/migrations --shadow-database-url \"postgres://klicker-prod:klicker@localhost:5432/shadow\"'", + "prisma:diff": "CONFIG=dev ../../util/_run_with_doppler.sh sh -lc 'prisma migrate diff --from-url \"$PROD_DATABASE_URL\" --to-migrations src/prisma/migrations --shadow-database-url \"$SHADOW_DATABASE_URL\"'"Run to verify no leftover secrets or diff scripts:
#!/bin/bash rg -n -C2 -P 'prisma\s+migrate\s+diff|postgres://[^@]+:[^@]+@' package.json packages/**/package.json.github/workflows/cypress-testing.yml (1)
208-211: Past note: JWT issuer vars should also use CYPRESS_ prefix for spec access.If you re-introduce JWT_ISSUER_* for tests, prefix them so Cypress.env('JWT_ISSUER_AUTH') works.
- JWT_ISSUER_AUTH: http://127.0.0.1:3010 - JWT_ISSUER_API: http://127.0.0.1:3000 + CYPRESS_JWT_ISSUER_AUTH: http://127.0.0.1:3010 + CYPRESS_JWT_ISSUER_API: http://127.0.0.1:3000Also applies to: 419-421
apps/auth/.env.production (1)
5-5: Align APP_ORIGIN_API to the canonical public API host (prevents issuer/audience and CORS drift).PWA prod uses https://api.klicker.uzh.ch; this file still points to backend-sls. For issuer verification rollout, keep a single canonical API origin.
-APP_ORIGIN_API=https://backend-sls.klicker.uzh.ch +APP_ORIGIN_API=https://api.klicker.uzh.ch
🧹 Nitpick comments (9)
packages/prisma/package.json (1)
42-45: Make scripts cross‑platform.Bare
CONFIG=dev <cmd>won’t work on Windows shells. Use cross-env (already a devDependency).- "prisma:migrate": "CONFIG=dev ../../util/_run_with_doppler.sh prisma migrate dev", + "prisma:migrate": "cross-env CONFIG=dev ../../util/_run_with_doppler.sh prisma migrate dev", - "prisma:push": "CONFIG=dev ../../util/_run_with_doppler.sh pnpm run prisma:push:raw", + "prisma:push": "cross-env CONFIG=dev ../../util/_run_with_doppler.sh pnpm run prisma:push:raw", - "prisma:push:qa": "CONFIG=stg ../../util/_run_with_doppler.sh pnpm run prisma:push:raw", + "prisma:push:qa": "cross-env CONFIG=stg ../../util/_run_with_doppler.sh pnpm run prisma:push:raw"Apply similarly to other
CONFIG=...scripts for parity.apps/hatchet-worker-general/.env.example (1)
8-15: Drop quotes and (optionally) sort keys for linter compliance.dotenv-linter flags quotes and ordering. Remove quotes and, if you care about a clean lint pass, sort alphabetically.
-APP_ORIGIN_AUTH="http://127.0.0.1:3010" -APP_ORIGIN_API="http://127.0.0.1:3000" -APP_ORIGIN_LTI="http://127.0.0.1:4000" -APP_ORIGIN_PWA="http://127.0.0.1:3001" -APP_ORIGIN_MANAGE="http://127.0.0.1:3002" -APP_ORIGIN_CONTROL="http://127.0.0.1:3003" -APP_ORIGIN_ASSESSMENT_API="http://127.0.0.1:3000" -APP_ORIGIN_ASSESSMENT_PWA="http://127.0.0.1:3001" +APP_ORIGIN_API=http://127.0.0.1:3000 +APP_ORIGIN_ASSESSMENT_API=http://127.0.0.1:3000 +APP_ORIGIN_ASSESSMENT_PWA=http://127.0.0.1:3001 +APP_ORIGIN_AUTH=http://127.0.0.1:3010 +APP_ORIGIN_CONTROL=http://127.0.0.1:3003 +APP_ORIGIN_LTI=http://127.0.0.1:4000 +APP_ORIGIN_MANAGE=http://127.0.0.1:3002 +APP_ORIGIN_PWA=http://127.0.0.1:3001apps/auth/.env.development (1)
5-12: Keep origins consistent: ordering nit + confirm interpolation works in this app.
- Optional: sort keys for linter.
- Please confirm dotenv expansion is enabled here so $APP_ORIGIN_* references resolve at runtime.
-APP_ORIGIN_API=https://api.klicker.com -APP_ORIGIN_PWA=https://pwa.klicker.com -APP_ORIGIN_MANAGE=https://manage.klicker.com -APP_ORIGIN_CONTROL=https://control.klicker.com -APP_ORIGIN_LTI=https://lti.klicker.com -APP_ORIGIN_AUTH=https://auth.klicker.com -APP_ORIGIN_ASSESSMENT_API=https://assessment-api.klicker.com -APP_ORIGIN_ASSESSMENT_PWA=https://assessment.klicker.com +APP_ORIGIN_API=https://api.klicker.com +APP_ORIGIN_ASSESSMENT_API=https://assessment-api.klicker.com +APP_ORIGIN_ASSESSMENT_PWA=https://assessment.klicker.com +APP_ORIGIN_AUTH=https://auth.klicker.com +APP_ORIGIN_CONTROL=https://control.klicker.com +APP_ORIGIN_LTI=https://lti.klicker.com +APP_ORIGIN_MANAGE=https://manage.klicker.com +APP_ORIGIN_PWA=https://pwa.klicker.comapps/hatchet-worker-response-processor/.env.example (1)
10-17: Remove quotes and optionally sort keys to placate linter.Same nit as other envs.
-APP_ORIGIN_AUTH="http://127.0.0.1:3010" -APP_ORIGIN_API="http://127.0.0.1:3000" -APP_ORIGIN_LTI="http://127.0.0.1:4000" -APP_ORIGIN_PWA="http://127.0.0.1:3001" -APP_ORIGIN_MANAGE="http://127.0.0.1:3002" -APP_ORIGIN_CONTROL="http://127.0.0.1:3003" -APP_ORIGIN_ASSESSMENT_API="http://127.0.0.1:3000" -APP_ORIGIN_ASSESSMENT_PWA="http://127.0.0.1:3001" +APP_ORIGIN_API=http://127.0.0.1:3000 +APP_ORIGIN_ASSESSMENT_API=http://127.0.0.1:3000 +APP_ORIGIN_ASSESSMENT_PWA=http://127.0.0.1:3001 +APP_ORIGIN_AUTH=http://127.0.0.1:3010 +APP_ORIGIN_CONTROL=http://127.0.0.1:3003 +APP_ORIGIN_LTI=http://127.0.0.1:4000 +APP_ORIGIN_MANAGE=http://127.0.0.1:3002 +APP_ORIGIN_PWA=http://127.0.0.1:3001apps/backend-docker/.env.cypress (2)
18-25: Unquote and (optionally) sort origins.Small linter cleanup.
-APP_ORIGIN_AUTH="http://127.0.0.1:3010" -APP_ORIGIN_API="http://127.0.0.1:3000" -APP_ORIGIN_LTI="http://127.0.0.1:4000" -APP_ORIGIN_PWA="http://127.0.0.1:3001" -APP_ORIGIN_MANAGE="http://127.0.0.1:3002" -APP_ORIGIN_CONTROL="http://127.0.0.1:3003" -APP_ORIGIN_ASSESSMENT_API="http://127.0.0.1:3000" -APP_ORIGIN_ASSESSMENT_PWA="http://127.0.0.1:3001" +APP_ORIGIN_API=http://127.0.0.1:3000 +APP_ORIGIN_ASSESSMENT_API=http://127.0.0.1:3000 +APP_ORIGIN_ASSESSMENT_PWA=http://127.0.0.1:3001 +APP_ORIGIN_AUTH=http://127.0.0.1:3010 +APP_ORIGIN_CONTROL=http://127.0.0.1:3003 +APP_ORIGIN_LTI=http://127.0.0.1:4000 +APP_ORIGIN_MANAGE=http://127.0.0.1:3002 +APP_ORIGIN_PWA=http://127.0.0.1:3001
17-25: Deprecate legacy domain env vars or document precedence (APP_ORIGIN is SSoT)apps/backend-docker/.env.cypress defines both legacy vars (API_DOMAIN, APP_SUBDOMAIN) and APP_ORIGIN — APP_ORIGIN already has "Canonical app origins (prefer these; issuers fall back to them)". Comment out or remove the legacy vars (or add a clear deprecation comment above them) and verify no scripts reference the legacy names before deleting.
-API_DOMAIN="127.0.0.1:3000" -APP_STUDENT_SUBDOMAIN="127.0.0.1:3001" -APP_MANAGE_SUBDOMAIN="127.0.0.1:3002" -APP_CONTROL_SUBDOMAIN="127.0.0.1:3003" +# Deprecated in favor of APP_ORIGIN_* (see below). Keep only if still referenced by legacy scripts. +# API_DOMAIN="127.0.0.1:3000" +# APP_STUDENT_SUBDOMAIN="127.0.0.1:3001" +# APP_MANAGE_SUBDOMAIN="127.0.0.1:3002" +# APP_CONTROL_SUBDOMAIN="127.0.0.1:3003"apps/auth/.env.qa (1)
5-12: Ordering nit and issuer consistency check.Optional: sort keys; also verify issuer checks elsewhere expect exactly https://auth.klicker-qa.bf-app.ch (no trailing slash).
apps/auth/.env.test (1)
8-15: Nice: no quotes; only nit is ordering to appease linter.If you want a clean lint run, sort keys alphabetically as below. Otherwise LGTM.
-APP_ORIGIN_API=http://127.0.0.1:3000 -APP_ORIGIN_PWA=http://127.0.0.1:3001 -APP_ORIGIN_MANAGE=http://127.0.0.1:3002 -APP_ORIGIN_CONTROL=http://127.0.0.1:3003 -APP_ORIGIN_LTI=http://127.0.0.1:4000 -APP_ORIGIN_AUTH=http://127.0.0.1:3010 -APP_ORIGIN_ASSESSMENT_API=http://127.0.0.1:3000 -APP_ORIGIN_ASSESSMENT_PWA=http://127.0.0.1:3001 +APP_ORIGIN_API=http://127.0.0.1:3000 +APP_ORIGIN_ASSESSMENT_API=http://127.0.0.1:3000 +APP_ORIGIN_ASSESSMENT_PWA=http://127.0.0.1:3001 +APP_ORIGIN_AUTH=http://127.0.0.1:3010 +APP_ORIGIN_CONTROL=http://127.0.0.1:3003 +APP_ORIGIN_LTI=http://127.0.0.1:4000 +APP_ORIGIN_MANAGE=http://127.0.0.1:3002 +APP_ORIGIN_PWA=http://127.0.0.1:3001apps/auth/.env.production (1)
6-12: Optional: sort keys to satisfy dotenv-linter noise.Pure housekeeping; keeps CI lint quiet.
APP_ORIGIN_API=https://api.klicker.uzh.ch -APP_ORIGIN_PWA=https://pwa.klicker.uzh.ch -APP_ORIGIN_MANAGE=https://manage.klicker.uzh.ch -APP_ORIGIN_CONTROL=https://control.klicker.uzh.ch -APP_ORIGIN_LTI=https://lti.klicker.uzh.ch -APP_ORIGIN_AUTH=https://auth.klicker.uzh.ch -APP_ORIGIN_ASSESSMENT_API=https://assessment-api.klicker.uzh.ch -APP_ORIGIN_ASSESSMENT_PWA=https://assessment.klicker.uzh.ch +APP_ORIGIN_ASSESSMENT_API=https://assessment-api.klicker.uzh.ch +APP_ORIGIN_ASSESSMENT_PWA=https://assessment.klicker.uzh.ch +APP_ORIGIN_AUTH=https://auth.klicker.uzh.ch +APP_ORIGIN_CONTROL=https://control.klicker.uzh.ch +APP_ORIGIN_LTI=https://lti.klicker.uzh.ch +APP_ORIGIN_MANAGE=https://manage.klicker.uzh.ch +APP_ORIGIN_PWA=https://pwa.klicker.uzh.ch @@ -NEXT_PUBLIC_PWA_URL=$APP_ORIGIN_PWA -NEXT_PUBLIC_MANAGE_URL=$APP_ORIGIN_MANAGE -NEXT_PUBLIC_CONTROL_URL=$APP_ORIGIN_CONTROL -NEXT_PUBLIC_AUTH_URL=$APP_ORIGIN_AUTH -NEXT_PUBLIC_LTI_URL=$APP_ORIGIN_LTI -NEXT_PUBLIC_ASSESSMENT_URL=$APP_ORIGIN_ASSESSMENT_PWA -NEXT_PUBLIC_ASSESSMENT_API_URL=$APP_ORIGIN_ASSESSMENT_API +NEXT_PUBLIC_ASSESSMENT_API_URL=$APP_ORIGIN_ASSESSMENT_API +NEXT_PUBLIC_ASSESSMENT_URL=$APP_ORIGIN_ASSESSMENT_PWA +NEXT_PUBLIC_AUTH_URL=$APP_ORIGIN_AUTH +NEXT_PUBLIC_CONTROL_URL=$APP_ORIGIN_CONTROL +NEXT_PUBLIC_LTI_URL=$APP_ORIGIN_LTI +NEXT_PUBLIC_MANAGE_URL=$APP_ORIGIN_MANAGE +NEXT_PUBLIC_PWA_URL=$APP_ORIGIN_PWAAlso applies to: 15-21
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (13)
.github/workflows/cypress-testing.yml(4 hunks).github/workflows/test-graphql.yml(1 hunks)apps/auth/.env.development(1 hunks)apps/auth/.env.production(1 hunks)apps/auth/.env.qa(1 hunks)apps/auth/.env.test(1 hunks)apps/auth/src/pages/api/auth/[...nextauth].ts(3 hunks)apps/backend-docker/.env.cypress(1 hunks)apps/backend-docker/.env.example(1 hunks)apps/hatchet-worker-general/.env.example(1 hunks)apps/hatchet-worker-response-processor/.env.example(1 hunks)apps/response-api/.env.example(1 hunks)packages/prisma/package.json(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- apps/auth/src/pages/api/auth/[...nextauth].ts
- .github/workflows/test-graphql.yml
🧰 Additional context used
🪛 dotenv-linter (3.3.0)
apps/auth/.env.development
[warning] 7-7: [UnorderedKey] The APP_ORIGIN_MANAGE key should go before the APP_ORIGIN_PWA key
(UnorderedKey)
[warning] 8-8: [UnorderedKey] The APP_ORIGIN_CONTROL key should go before the APP_ORIGIN_MANAGE key
(UnorderedKey)
[warning] 9-9: [UnorderedKey] The APP_ORIGIN_LTI key should go before the APP_ORIGIN_MANAGE key
(UnorderedKey)
[warning] 10-10: [UnorderedKey] The APP_ORIGIN_AUTH key should go before the APP_ORIGIN_CONTROL key
(UnorderedKey)
[warning] 11-11: [UnorderedKey] The APP_ORIGIN_ASSESSMENT_API key should go before the APP_ORIGIN_AUTH key
(UnorderedKey)
[warning] 12-12: [UnorderedKey] The APP_ORIGIN_ASSESSMENT_PWA key should go before the APP_ORIGIN_AUTH key
(UnorderedKey)
[warning] 15-15: [UnorderedKey] The NEXT_PUBLIC_MANAGE_URL key should go before the NEXT_PUBLIC_PWA_URL key
(UnorderedKey)
[warning] 16-16: [UnorderedKey] The NEXT_PUBLIC_CONTROL_URL key should go before the NEXT_PUBLIC_MANAGE_URL key
(UnorderedKey)
[warning] 17-17: [UnorderedKey] The NEXT_PUBLIC_AUTH_URL key should go before the NEXT_PUBLIC_CONTROL_URL key
(UnorderedKey)
[warning] 18-18: [UnorderedKey] The NEXT_PUBLIC_LTI_URL key should go before the NEXT_PUBLIC_MANAGE_URL key
(UnorderedKey)
[warning] 19-19: [UnorderedKey] The NEXT_PUBLIC_ASSESSMENT_URL key should go before the NEXT_PUBLIC_AUTH_URL key
(UnorderedKey)
[warning] 20-20: [UnorderedKey] The NEXT_PUBLIC_ASSESSMENT_API_URL key should go before the NEXT_PUBLIC_ASSESSMENT_URL key
(UnorderedKey)
apps/auth/.env.production
[warning] 7-7: [UnorderedKey] The APP_ORIGIN_MANAGE key should go before the APP_ORIGIN_PWA key
(UnorderedKey)
[warning] 8-8: [UnorderedKey] The APP_ORIGIN_CONTROL key should go before the APP_ORIGIN_MANAGE key
(UnorderedKey)
[warning] 9-9: [UnorderedKey] The APP_ORIGIN_LTI key should go before the APP_ORIGIN_MANAGE key
(UnorderedKey)
[warning] 10-10: [UnorderedKey] The APP_ORIGIN_AUTH key should go before the APP_ORIGIN_CONTROL key
(UnorderedKey)
[warning] 11-11: [UnorderedKey] The APP_ORIGIN_ASSESSMENT_API key should go before the APP_ORIGIN_AUTH key
(UnorderedKey)
[warning] 12-12: [UnorderedKey] The APP_ORIGIN_ASSESSMENT_PWA key should go before the APP_ORIGIN_AUTH key
(UnorderedKey)
[warning] 15-15: [UnorderedKey] The NEXT_PUBLIC_MANAGE_URL key should go before the NEXT_PUBLIC_PWA_URL key
(UnorderedKey)
[warning] 16-16: [UnorderedKey] The NEXT_PUBLIC_CONTROL_URL key should go before the NEXT_PUBLIC_MANAGE_URL key
(UnorderedKey)
[warning] 17-17: [UnorderedKey] The NEXT_PUBLIC_AUTH_URL key should go before the NEXT_PUBLIC_CONTROL_URL key
(UnorderedKey)
[warning] 18-18: [UnorderedKey] The NEXT_PUBLIC_LTI_URL key should go before the NEXT_PUBLIC_MANAGE_URL key
(UnorderedKey)
[warning] 19-19: [UnorderedKey] The NEXT_PUBLIC_ASSESSMENT_URL key should go before the NEXT_PUBLIC_AUTH_URL key
(UnorderedKey)
[warning] 20-20: [UnorderedKey] The NEXT_PUBLIC_ASSESSMENT_API_URL key should go before the NEXT_PUBLIC_ASSESSMENT_URL key
(UnorderedKey)
apps/auth/.env.qa
[warning] 7-7: [UnorderedKey] The APP_ORIGIN_MANAGE key should go before the APP_ORIGIN_PWA key
(UnorderedKey)
[warning] 8-8: [UnorderedKey] The APP_ORIGIN_CONTROL key should go before the APP_ORIGIN_MANAGE key
(UnorderedKey)
[warning] 9-9: [UnorderedKey] The APP_ORIGIN_LTI key should go before the APP_ORIGIN_MANAGE key
(UnorderedKey)
[warning] 10-10: [UnorderedKey] The APP_ORIGIN_AUTH key should go before the APP_ORIGIN_CONTROL key
(UnorderedKey)
[warning] 11-11: [UnorderedKey] The APP_ORIGIN_ASSESSMENT_API key should go before the APP_ORIGIN_AUTH key
(UnorderedKey)
[warning] 12-12: [UnorderedKey] The APP_ORIGIN_ASSESSMENT_PWA key should go before the APP_ORIGIN_AUTH key
(UnorderedKey)
[warning] 15-15: [UnorderedKey] The NEXT_PUBLIC_MANAGE_URL key should go before the NEXT_PUBLIC_PWA_URL key
(UnorderedKey)
[warning] 16-16: [UnorderedKey] The NEXT_PUBLIC_CONTROL_URL key should go before the NEXT_PUBLIC_MANAGE_URL key
(UnorderedKey)
[warning] 17-17: [UnorderedKey] The NEXT_PUBLIC_AUTH_URL key should go before the NEXT_PUBLIC_CONTROL_URL key
(UnorderedKey)
[warning] 18-18: [UnorderedKey] The NEXT_PUBLIC_LTI_URL key should go before the NEXT_PUBLIC_MANAGE_URL key
(UnorderedKey)
[warning] 19-19: [UnorderedKey] The NEXT_PUBLIC_ASSESSMENT_URL key should go before the NEXT_PUBLIC_AUTH_URL key
(UnorderedKey)
[warning] 20-20: [UnorderedKey] The NEXT_PUBLIC_ASSESSMENT_API_URL key should go before the NEXT_PUBLIC_ASSESSMENT_URL key
(UnorderedKey)
apps/auth/.env.test
[warning] 6-6: [UnorderedKey] The APP_SECRET key should go before the DATABASE_URL key
(UnorderedKey)
[warning] 10-10: [UnorderedKey] The APP_ORIGIN_MANAGE key should go before the APP_ORIGIN_PWA key
(UnorderedKey)
[warning] 11-11: [UnorderedKey] The APP_ORIGIN_CONTROL key should go before the APP_ORIGIN_MANAGE key
(UnorderedKey)
[warning] 12-12: [UnorderedKey] The APP_ORIGIN_LTI key should go before the APP_ORIGIN_MANAGE key
(UnorderedKey)
[warning] 13-13: [UnorderedKey] The APP_ORIGIN_AUTH key should go before the APP_ORIGIN_CONTROL key
(UnorderedKey)
[warning] 14-14: [UnorderedKey] The APP_ORIGIN_ASSESSMENT_API key should go before the APP_ORIGIN_AUTH key
(UnorderedKey)
[warning] 15-15: [UnorderedKey] The APP_ORIGIN_ASSESSMENT_PWA key should go before the APP_ORIGIN_AUTH key
(UnorderedKey)
[warning] 18-18: [UnorderedKey] The NEXT_PUBLIC_MANAGE_URL key should go before the NEXT_PUBLIC_PWA_URL key
(UnorderedKey)
[warning] 19-19: [UnorderedKey] The NEXT_PUBLIC_CONTROL_URL key should go before the NEXT_PUBLIC_MANAGE_URL key
(UnorderedKey)
[warning] 20-20: [UnorderedKey] The NEXT_PUBLIC_AUTH_URL key should go before the NEXT_PUBLIC_CONTROL_URL key
(UnorderedKey)
[warning] 21-21: [UnorderedKey] The NEXT_PUBLIC_LTI_URL key should go before the NEXT_PUBLIC_MANAGE_URL key
(UnorderedKey)
[warning] 22-22: [UnorderedKey] The NEXT_PUBLIC_ASSESSMENT_URL key should go before the NEXT_PUBLIC_AUTH_URL key
(UnorderedKey)
[warning] 23-23: [UnorderedKey] The NEXT_PUBLIC_ASSESSMENT_API_URL key should go before the NEXT_PUBLIC_ASSESSMENT_URL key
(UnorderedKey)
apps/backend-docker/.env.cypress
[warning] 18-18: [QuoteCharacter] The value has quote characters (', ")
(QuoteCharacter)
[warning] 19-19: [QuoteCharacter] The value has quote characters (', ")
(QuoteCharacter)
[warning] 19-19: [UnorderedKey] The APP_ORIGIN_API key should go before the APP_ORIGIN_AUTH key
(UnorderedKey)
[warning] 20-20: [QuoteCharacter] The value has quote characters (', ")
(QuoteCharacter)
[warning] 21-21: [QuoteCharacter] The value has quote characters (', ")
(QuoteCharacter)
[warning] 22-22: [QuoteCharacter] The value has quote characters (', ")
(QuoteCharacter)
[warning] 22-22: [UnorderedKey] The APP_ORIGIN_MANAGE key should go before the APP_ORIGIN_PWA key
(UnorderedKey)
[warning] 23-23: [QuoteCharacter] The value has quote characters (', ")
(QuoteCharacter)
[warning] 23-23: [UnorderedKey] The APP_ORIGIN_CONTROL key should go before the APP_ORIGIN_LTI key
(UnorderedKey)
[warning] 24-24: [QuoteCharacter] The value has quote characters (', ")
(QuoteCharacter)
[warning] 24-24: [UnorderedKey] The APP_ORIGIN_ASSESSMENT_API key should go before the APP_ORIGIN_AUTH key
(UnorderedKey)
[warning] 25-25: [QuoteCharacter] The value has quote characters (', ")
(QuoteCharacter)
[warning] 25-25: [UnorderedKey] The APP_ORIGIN_ASSESSMENT_PWA key should go before the APP_ORIGIN_AUTH key
(UnorderedKey)
apps/backend-docker/.env.example
[warning] 18-18: [QuoteCharacter] The value has quote characters (', ")
(QuoteCharacter)
[warning] 19-19: [QuoteCharacter] The value has quote characters (', ")
(QuoteCharacter)
[warning] 19-19: [UnorderedKey] The APP_ORIGIN_API key should go before the APP_ORIGIN_AUTH key
(UnorderedKey)
[warning] 20-20: [QuoteCharacter] The value has quote characters (', ")
(QuoteCharacter)
[warning] 21-21: [QuoteCharacter] The value has quote characters (', ")
(QuoteCharacter)
[warning] 22-22: [QuoteCharacter] The value has quote characters (', ")
(QuoteCharacter)
[warning] 22-22: [UnorderedKey] The APP_ORIGIN_MANAGE key should go before the APP_ORIGIN_PWA key
(UnorderedKey)
[warning] 23-23: [QuoteCharacter] The value has quote characters (', ")
(QuoteCharacter)
[warning] 23-23: [UnorderedKey] The APP_ORIGIN_CONTROL key should go before the APP_ORIGIN_LTI key
(UnorderedKey)
[warning] 24-24: [QuoteCharacter] The value has quote characters (', ")
(QuoteCharacter)
[warning] 24-24: [UnorderedKey] The APP_ORIGIN_ASSESSMENT_API key should go before the APP_ORIGIN_AUTH key
(UnorderedKey)
[warning] 25-25: [QuoteCharacter] The value has quote characters (', ")
(QuoteCharacter)
[warning] 25-25: [UnorderedKey] The APP_ORIGIN_ASSESSMENT_PWA key should go before the APP_ORIGIN_AUTH key
(UnorderedKey)
apps/hatchet-worker-general/.env.example
[warning] 8-8: [QuoteCharacter] The value has quote characters (', ")
(QuoteCharacter)
[warning] 9-9: [QuoteCharacter] The value has quote characters (', ")
(QuoteCharacter)
[warning] 9-9: [UnorderedKey] The APP_ORIGIN_API key should go before the APP_ORIGIN_AUTH key
(UnorderedKey)
[warning] 10-10: [QuoteCharacter] The value has quote characters (', ")
(QuoteCharacter)
[warning] 11-11: [QuoteCharacter] The value has quote characters (', ")
(QuoteCharacter)
[warning] 12-12: [QuoteCharacter] The value has quote characters (', ")
(QuoteCharacter)
[warning] 12-12: [UnorderedKey] The APP_ORIGIN_MANAGE key should go before the APP_ORIGIN_PWA key
(UnorderedKey)
[warning] 13-13: [QuoteCharacter] The value has quote characters (', ")
(QuoteCharacter)
[warning] 13-13: [UnorderedKey] The APP_ORIGIN_CONTROL key should go before the APP_ORIGIN_LTI key
(UnorderedKey)
[warning] 14-14: [QuoteCharacter] The value has quote characters (', ")
(QuoteCharacter)
[warning] 14-14: [UnorderedKey] The APP_ORIGIN_ASSESSMENT_API key should go before the APP_ORIGIN_AUTH key
(UnorderedKey)
[warning] 15-15: [QuoteCharacter] The value has quote characters (', ")
(QuoteCharacter)
[warning] 15-15: [UnorderedKey] The APP_ORIGIN_ASSESSMENT_PWA key should go before the APP_ORIGIN_AUTH key
(UnorderedKey)
apps/hatchet-worker-response-processor/.env.example
[warning] 10-10: [QuoteCharacter] The value has quote characters (', ")
(QuoteCharacter)
[warning] 11-11: [QuoteCharacter] The value has quote characters (', ")
(QuoteCharacter)
[warning] 11-11: [UnorderedKey] The APP_ORIGIN_API key should go before the APP_ORIGIN_AUTH key
(UnorderedKey)
[warning] 12-12: [QuoteCharacter] The value has quote characters (', ")
(QuoteCharacter)
[warning] 13-13: [QuoteCharacter] The value has quote characters (', ")
(QuoteCharacter)
[warning] 14-14: [QuoteCharacter] The value has quote characters (', ")
(QuoteCharacter)
[warning] 14-14: [UnorderedKey] The APP_ORIGIN_MANAGE key should go before the APP_ORIGIN_PWA key
(UnorderedKey)
[warning] 15-15: [QuoteCharacter] The value has quote characters (', ")
(QuoteCharacter)
[warning] 15-15: [UnorderedKey] The APP_ORIGIN_CONTROL key should go before the APP_ORIGIN_LTI key
(UnorderedKey)
[warning] 16-16: [QuoteCharacter] The value has quote characters (', ")
(QuoteCharacter)
[warning] 16-16: [UnorderedKey] The APP_ORIGIN_ASSESSMENT_API key should go before the APP_ORIGIN_AUTH key
(UnorderedKey)
[warning] 17-17: [QuoteCharacter] The value has quote characters (', ")
(QuoteCharacter)
[warning] 17-17: [UnorderedKey] The APP_ORIGIN_ASSESSMENT_PWA key should go before the APP_ORIGIN_AUTH key
(UnorderedKey)
apps/response-api/.env.example
[warning] 9-9: [QuoteCharacter] The value has quote characters (', ")
(QuoteCharacter)
[warning] 10-10: [QuoteCharacter] The value has quote characters (', ")
(QuoteCharacter)
[warning] 10-10: [UnorderedKey] The APP_ORIGIN_API key should go before the APP_ORIGIN_AUTH key
(UnorderedKey)
[warning] 11-11: [QuoteCharacter] The value has quote characters (', ")
(QuoteCharacter)
[warning] 12-12: [QuoteCharacter] The value has quote characters (', ")
(QuoteCharacter)
[warning] 13-13: [QuoteCharacter] The value has quote characters (', ")
(QuoteCharacter)
[warning] 13-13: [UnorderedKey] The APP_ORIGIN_MANAGE key should go before the APP_ORIGIN_PWA key
(UnorderedKey)
[warning] 14-14: [QuoteCharacter] The value has quote characters (', ")
(QuoteCharacter)
[warning] 14-14: [UnorderedKey] The APP_ORIGIN_CONTROL key should go before the APP_ORIGIN_LTI key
(UnorderedKey)
[warning] 15-15: [QuoteCharacter] The value has quote characters (', ")
(QuoteCharacter)
[warning] 15-15: [UnorderedKey] The APP_ORIGIN_ASSESSMENT_API key should go before the APP_ORIGIN_AUTH key
(UnorderedKey)
[warning] 16-16: [QuoteCharacter] The value has quote characters (', ")
(QuoteCharacter)
[warning] 16-16: [UnorderedKey] The APP_ORIGIN_ASSESSMENT_PWA key should go before the APP_ORIGIN_AUTH key
(UnorderedKey)
🪛 Checkov (3.2.334)
packages/prisma/package.json
[medium] 41-42: Basic Auth Credentials
(CKV_SECRET_4)
🔇 Additional comments (7)
packages/prisma/package.json (2)
37-39: Good move to centralize env handling via the Doppler wrapper.These deploy scripts now avoid leaking env and improve consistency.
58-60: LGTM on prisma:studio routing through the wrapper.Consistent handling across dev/prod/qa.
apps/auth/.env.development (1)
14-22: Public URL exports: verify no trailing slashes and consistent issuer usage.Looks good; just confirm consumers don’t require a trailing slash and that JWT issuer equals APP_ORIGIN_AUTH exactly (no slash).
apps/auth/.env.qa (1)
14-22: NEXT_PUBLIC_ derivations look good.*No issues; nice consolidation to a single source of truth.
apps/auth/.env.test (1)
17-25: Public URL exports and NEXTAUTH_URL look correct for tests.Issuer should match APP_ORIGIN_AUTH exactly; no trailing slash.
.github/workflows/cypress-testing.yml (1)
183-191: ```shell
#!/bin/bash
set -euo pipefailPrint workflow snippet around the lines mentioned in the review (if file exists)
printf "\n=== .github/workflows/cypress-testing.yml (lines ~170-220) ===\n"
sed -n '160,220p' .github/workflows/cypress-testing.yml || trueFind all APP_ORIGIN_* occurrences (show context)
printf "\n=== Occurrences of APP_ORIGIN_* ===\n"
rg -n --hidden --no-ignore -S '\bAPP_ORIGIN_[A-Z0-9_]+\b' -C2 || trueFind any existing CYPRESS_APP_ORIGIN_* variables or CYPRESS_ usage
printf "\n=== Occurrences of CYPRESS_APP_ORIGIN or CYPRESS_ env usage ===\n"
rg -n --hidden --no-ignore -S '\bCYPRESS_APP_ORIGIN_[A-Z0-9_]+\b|\bCYPRESS_[A-Z0-9_]+\b' -C2 || trueFind where tests/config read env vars via Cypress.env or process.env
printf "\n=== Usage of Cypress.env / Cypress.config().env / process.env.APP_ORIGIN_ ===\n"
rg -n --hidden --no-ignore -S 'Cypress.env|Cypress.config().env|Cypress.config(|process.env.(APP_ORIGIN|APP_ORIGIN_API|APP_ORIGIN_AUTH|APP_ORIGIN_LTI|APP_ORIGIN_PWA|APP_ORIGIN_MANAGE|APP_ORIGIN_CONTROL|APP_ORIGIN_ASSESSMENT_API|APP_ORIGIN_ASSESSMENT_PWA)\b' -C2 || trueFind cypress config files (cypress.config.ts/js or cypress.json) and show them
printf "\n=== cypress config files ===\n"
rg -n --hidden --no-ignore -S 'cypress.config.(ts|js|mjs|cjs)|cypress.json' -g '!/node_modules/' -C0 || true
rg -n --hidden --no-ignore -S 'defineConfig(|module.exports\s*=' -g '!/node_modules/' -C2 || true</blockquote></details> <details> <summary>apps/auth/.env.production (1)</summary><blockquote> `14-22`: **Verify dotenv var expansion is enabled for production (.env.production uses $VAR references)** apps/auth/.env.production uses $APP_ORIGIN_* expansions (e.g. NEXT_PUBLIC_PWA_URL=$APP_ORIGIN_PWA). pnpm-lock.yaml contains dotenv-expand, but I did not find an explicit invocation that guarantees expansion for apps/auth at build/start — check apps/auth/package.json build/start flow, any next.config.* or the @klicker-uzh/next-config package for dotenv/dotenv-expand usage; if absent, enable dotenv-expand in the loader (require('dotenv').config(); require('dotenv-expand')(process.env)) or inline resolved values during CI/CD. </blockquote></details> </blockquote></details> </details> <!-- This is an auto-generated comment by CodeRabbit for review status -->
| # Canonical app origins (prefer these; issuers fall back to them) | ||
| APP_ORIGIN_AUTH="http://127.0.0.1:3010" | ||
| APP_ORIGIN_API="http://127.0.0.1:3000" | ||
| APP_ORIGIN_LTI="http://127.0.0.1:4000" | ||
| APP_ORIGIN_PWA="http://127.0.0.1:3001" | ||
| APP_ORIGIN_MANAGE="http://127.0.0.1:3002" | ||
| APP_ORIGIN_CONTROL="http://127.0.0.1:3003" | ||
| APP_ORIGIN_ASSESSMENT_API="http://127.0.0.1:3000" | ||
| APP_ORIGIN_ASSESSMENT_PWA="http://127.0.0.1:3001" |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Same nits as cypress env: prefer APP_ORIGIN_ as SSoT and unquote.*
- Consider commenting out legacy API_DOMAIN/APP_*_SUBDOMAIN here too (if unused).
- Remove quotes and optionally sort.
-APP_ORIGIN_AUTH="http://127.0.0.1:3010"
-APP_ORIGIN_API="http://127.0.0.1:3000"
-APP_ORIGIN_LTI="http://127.0.0.1:4000"
-APP_ORIGIN_PWA="http://127.0.0.1:3001"
-APP_ORIGIN_MANAGE="http://127.0.0.1:3002"
-APP_ORIGIN_CONTROL="http://127.0.0.1:3003"
-APP_ORIGIN_ASSESSMENT_API="http://127.0.0.1:3000"
-APP_ORIGIN_ASSESSMENT_PWA="http://127.0.0.1:3001"
+APP_ORIGIN_API=http://127.0.0.1:3000
+APP_ORIGIN_ASSESSMENT_API=http://127.0.0.1:3000
+APP_ORIGIN_ASSESSMENT_PWA=http://127.0.0.1:3001
+APP_ORIGIN_AUTH=http://127.0.0.1:3010
+APP_ORIGIN_CONTROL=http://127.0.0.1:3003
+APP_ORIGIN_LTI=http://127.0.0.1:4000
+APP_ORIGIN_MANAGE=http://127.0.0.1:3002
+APP_ORIGIN_PWA=http://127.0.0.1:3001📝 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.
| # Canonical app origins (prefer these; issuers fall back to them) | |
| APP_ORIGIN_AUTH="http://127.0.0.1:3010" | |
| APP_ORIGIN_API="http://127.0.0.1:3000" | |
| APP_ORIGIN_LTI="http://127.0.0.1:4000" | |
| APP_ORIGIN_PWA="http://127.0.0.1:3001" | |
| APP_ORIGIN_MANAGE="http://127.0.0.1:3002" | |
| APP_ORIGIN_CONTROL="http://127.0.0.1:3003" | |
| APP_ORIGIN_ASSESSMENT_API="http://127.0.0.1:3000" | |
| APP_ORIGIN_ASSESSMENT_PWA="http://127.0.0.1:3001" | |
| # Canonical app origins (prefer these; issuers fall back to them) | |
| APP_ORIGIN_API=http://127.0.0.1:3000 | |
| APP_ORIGIN_ASSESSMENT_API=http://127.0.0.1:3000 | |
| APP_ORIGIN_ASSESSMENT_PWA=http://127.0.0.1:3001 | |
| APP_ORIGIN_AUTH=http://127.0.0.1:3010 | |
| APP_ORIGIN_CONTROL=http://127.0.0.1:3003 | |
| APP_ORIGIN_LTI=http://127.0.0.1:4000 | |
| APP_ORIGIN_MANAGE=http://127.0.0.1:3002 | |
| APP_ORIGIN_PWA=http://127.0.0.1:3001 |
🧰 Tools
🪛 dotenv-linter (3.3.0)
[warning] 18-18: [QuoteCharacter] The value has quote characters (', ")
(QuoteCharacter)
[warning] 19-19: [QuoteCharacter] The value has quote characters (', ")
(QuoteCharacter)
[warning] 19-19: [UnorderedKey] The APP_ORIGIN_API key should go before the APP_ORIGIN_AUTH key
(UnorderedKey)
[warning] 20-20: [QuoteCharacter] The value has quote characters (', ")
(QuoteCharacter)
[warning] 21-21: [QuoteCharacter] The value has quote characters (', ")
(QuoteCharacter)
[warning] 22-22: [QuoteCharacter] The value has quote characters (', ")
(QuoteCharacter)
[warning] 22-22: [UnorderedKey] The APP_ORIGIN_MANAGE key should go before the APP_ORIGIN_PWA key
(UnorderedKey)
[warning] 23-23: [QuoteCharacter] The value has quote characters (', ")
(QuoteCharacter)
[warning] 23-23: [UnorderedKey] The APP_ORIGIN_CONTROL key should go before the APP_ORIGIN_LTI key
(UnorderedKey)
[warning] 24-24: [QuoteCharacter] The value has quote characters (', ")
(QuoteCharacter)
[warning] 24-24: [UnorderedKey] The APP_ORIGIN_ASSESSMENT_API key should go before the APP_ORIGIN_AUTH key
(UnorderedKey)
[warning] 25-25: [QuoteCharacter] The value has quote characters (', ")
(QuoteCharacter)
[warning] 25-25: [UnorderedKey] The APP_ORIGIN_ASSESSMENT_PWA key should go before the APP_ORIGIN_AUTH key
(UnorderedKey)
🤖 Prompt for AI Agents
In apps/backend-docker/.env.example around lines 17 to 25, the APP_ORIGIN_*
entries are quoted and legacy API_DOMAIN/APP_*_SUBDOMAIN variables are still
present; update this file to use the APP_ORIGIN_* variables as the single source
of truth by removing the surrounding quotes from each APP_ORIGIN_* value,
comment out (or remove) any unused legacy API_DOMAIN/APP_*_SUBDOMAIN entries,
and optionally sort the APP_ORIGIN_* lines for clarity while preserving the same
URLs.
| APP_ORIGIN_AUTH="http://127.0.0.1:3010" | ||
| APP_ORIGIN_API="http://127.0.0.1:3000" | ||
| APP_ORIGIN_LTI="http://127.0.0.1:4000" | ||
| APP_ORIGIN_PWA="http://127.0.0.1:3001" | ||
| APP_ORIGIN_MANAGE="http://127.0.0.1:3002" | ||
| APP_ORIGIN_CONTROL="http://127.0.0.1:3003" | ||
| APP_ORIGIN_ASSESSMENT_API="http://127.0.0.1:3000" | ||
| APP_ORIGIN_ASSESSMENT_PWA="http://127.0.0.1:3001" |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Unquote URLs and consider deriving CORS from APP_ORIGIN_ to avoid drift.*
- Remove quotes to satisfy linter.
- Recommend building CORS_ALLOWED_ORIGINS from APP_ORIGIN_* at runtime/config to keep them in sync.
-APP_ORIGIN_AUTH="http://127.0.0.1:3010"
-APP_ORIGIN_API="http://127.0.0.1:3000"
-APP_ORIGIN_LTI="http://127.0.0.1:4000"
-APP_ORIGIN_PWA="http://127.0.0.1:3001"
-APP_ORIGIN_MANAGE="http://127.0.0.1:3002"
-APP_ORIGIN_CONTROL="http://127.0.0.1:3003"
-APP_ORIGIN_ASSESSMENT_API="http://127.0.0.1:3000"
-APP_ORIGIN_ASSESSMENT_PWA="http://127.0.0.1:3001"
+APP_ORIGIN_API=http://127.0.0.1:3000
+APP_ORIGIN_ASSESSMENT_API=http://127.0.0.1:3000
+APP_ORIGIN_ASSESSMENT_PWA=http://127.0.0.1:3001
+APP_ORIGIN_AUTH=http://127.0.0.1:3010
+APP_ORIGIN_CONTROL=http://127.0.0.1:3003
+APP_ORIGIN_LTI=http://127.0.0.1:4000
+APP_ORIGIN_MANAGE=http://127.0.0.1:3002
+APP_ORIGIN_PWA=http://127.0.0.1:3001📝 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.
| APP_ORIGIN_AUTH="http://127.0.0.1:3010" | |
| APP_ORIGIN_API="http://127.0.0.1:3000" | |
| APP_ORIGIN_LTI="http://127.0.0.1:4000" | |
| APP_ORIGIN_PWA="http://127.0.0.1:3001" | |
| APP_ORIGIN_MANAGE="http://127.0.0.1:3002" | |
| APP_ORIGIN_CONTROL="http://127.0.0.1:3003" | |
| APP_ORIGIN_ASSESSMENT_API="http://127.0.0.1:3000" | |
| APP_ORIGIN_ASSESSMENT_PWA="http://127.0.0.1:3001" | |
| APP_ORIGIN_API=http://127.0.0.1:3000 | |
| APP_ORIGIN_ASSESSMENT_API=http://127.0.0.1:3000 | |
| APP_ORIGIN_ASSESSMENT_PWA=http://127.0.0.1:3001 | |
| APP_ORIGIN_AUTH=http://127.0.0.1:3010 | |
| APP_ORIGIN_CONTROL=http://127.0.0.1:3003 | |
| APP_ORIGIN_LTI=http://127.0.0.1:4000 | |
| APP_ORIGIN_MANAGE=http://127.0.0.1:3002 | |
| APP_ORIGIN_PWA=http://127.0.0.1:3001 |
🧰 Tools
🪛 dotenv-linter (3.3.0)
[warning] 9-9: [QuoteCharacter] The value has quote characters (', ")
(QuoteCharacter)
[warning] 10-10: [QuoteCharacter] The value has quote characters (', ")
(QuoteCharacter)
[warning] 10-10: [UnorderedKey] The APP_ORIGIN_API key should go before the APP_ORIGIN_AUTH key
(UnorderedKey)
[warning] 11-11: [QuoteCharacter] The value has quote characters (', ")
(QuoteCharacter)
[warning] 12-12: [QuoteCharacter] The value has quote characters (', ")
(QuoteCharacter)
[warning] 13-13: [QuoteCharacter] The value has quote characters (', ")
(QuoteCharacter)
[warning] 13-13: [UnorderedKey] The APP_ORIGIN_MANAGE key should go before the APP_ORIGIN_PWA key
(UnorderedKey)
[warning] 14-14: [QuoteCharacter] The value has quote characters (', ")
(QuoteCharacter)
[warning] 14-14: [UnorderedKey] The APP_ORIGIN_CONTROL key should go before the APP_ORIGIN_LTI key
(UnorderedKey)
[warning] 15-15: [QuoteCharacter] The value has quote characters (', ")
(QuoteCharacter)
[warning] 15-15: [UnorderedKey] The APP_ORIGIN_ASSESSMENT_API key should go before the APP_ORIGIN_AUTH key
(UnorderedKey)
[warning] 16-16: [QuoteCharacter] The value has quote characters (', ")
(QuoteCharacter)
[warning] 16-16: [UnorderedKey] The APP_ORIGIN_ASSESSMENT_PWA key should go before the APP_ORIGIN_AUTH key
(UnorderedKey)
🤖 Prompt for AI Agents
In apps/response-api/.env.example around lines 9–16 the APP_ORIGIN_* values are
quoted which fails the linter and may produce unexpected parsing; remove the
surrounding quotes so each value is an unquoted URL (e.g.,
APP_ORIGIN_AUTH=http://127.0.0.1:3010) and, in the application config, avoid
duplicating CORS_ALLOWED_ORIGINS by deriving it at runtime from the APP_ORIGIN_*
environment variables (collect the APP_ORIGIN_* keys, filter undefined values,
and use that array as the CORS origins), optionally keeping a single fallback
CORS_ALLOWED_ORIGINS env var for overrides.
| "prisma:reset": "CONFIG=dev ../../util/_run_with_doppler.sh pnpm run prisma:reset:raw", | ||
| "prisma:reset:qa": "CONFIG=stg ../../util/_run_with_doppler.sh pnpm run prisma:reset:raw", | ||
| "prisma:reset:raw": "prisma migrate reset --skip-seed", | ||
| "prisma:reset:seed": "doppler run --config dev -- prisma migrate reset", | ||
| "prisma:reset:yes": "doppler run --config dev -- pnpm run prisma:reset:raw -f", | ||
| "prisma:reset:seed": "CONFIG=dev ../../util/_run_with_doppler.sh prisma migrate reset", | ||
| "prisma:reset:yes": "CONFIG=dev ../../util/_run_with_doppler.sh pnpm run prisma:reset:raw -f", |
There was a problem hiding this comment.
💡 Verification agent
❓ Verification inconclusive
Guard destructive reset commands.
Even with CONFIG=dev, add a hard check in the wrapper to refuse reset unless CONFIG in an allowlist (e.g., dev, stg on CI sandboxes).
I can draft a wrapper snippet that exits non‑zero if CMD =~ /(migrate reset|db push)/ and CONFIG=prd.
Block destructive DB commands in the wrapper (util/_run_with_doppler.sh). Abort (exit non‑zero) when the invoked CMD matches /(migrate reset|db push)/ unless CONFIG is in an allowlist (e.g., dev, stg); protects packages/prisma/package.json (lines 47–51).
rschlaefli
changed the title
1 participant
Add to signed JWTs -> gradually implement verification, should not break anything like this
Summary by CodeRabbit