Skip to content

chore(quality): adopt Biome + Knip + Gitleaks tooling - #5186

Draft
rschlaefli wants to merge 8 commits into
v3from
claude/klicker-uzh-repo-quality-17a1bb
Draft

chore(quality): adopt Biome + Knip + Gitleaks tooling#5186
rschlaefli wants to merge 8 commits into
v3from
claude/klicker-uzh-repo-quality-17a1bb

Conversation

@rschlaefli

Copy link
Copy Markdown
Member

What This Adds

Migrates the repo's code-quality tooling and adds two new gates. Executed as a plan plus six phases plus one security fix (8 commits).

  1. Biome 2.5.2 is now the formatter and general linter for code (TS/JS/JSON/CSS). Prettier stays for Markdown/YAML and the playwright/ + cypress/ e2e suites, because Biome mangles Playwright test.describe.serial() chains. ESLint stays only as the Next.js safety net.
  2. Knip 6.24.0 gets a real config and reports unused files, deps, and exports; two verified dead items were removed.
  3. Gitleaks 8.30.1 secret scanning runs via a local husky pre-commit hook (it skips gracefully when the binary is absent) and a blocking CI job.
  4. New advisory CI jobs for Biome lint and Knip, non-blocking during the migration. check-format, check-types, check-syncpack, and check-gitleaks are blocking.
  5. Docs (AGENTS.md, engineering wiki, plan) updated to match.
  6. Security fix: removed a pre-existing live staging ADMIN JWT that was hardcoded in util/load-test/k6.js.

How It Works

  • Formatter split by directory, not file type. biome.json files.includes excludes playwright/ + cypress/ (and generated files). The format/format:check scripts and .lintstagedrc.mjs route non-e2e code to Biome and e2e + md/yaml to Prettier. The lint-staged config is check-only (run pnpm format to fix) and skips symlinks.
  • Versions use ~ (tilde) ranges to satisfy the repo's syncpack semver-group convention. The lockfile still pins exact 2.5.2/6.24.0, and the pnpm minimumReleaseAge guard blocks in-range drift, so no version floats. The supply-chain guard was left untouched.
  • CI: check-lint.yml gains an advisory biome lint step (continue-on-error) before the blocking Turbo/ESLint pass. New check-knip.yml runs knip as advisory. New check-gitleaks.yml installs the MIT gitleaks binary directly (gitleaks-action@v2 requires a GITLEAKS_LICENSE for org-owned repos) and runs a blocking full-tree scan. Both new workflows set least-privilege permissions: contents: read.
  • check-format.yml is unchanged: it already delegates to the format:check script, which now runs Biome plus Prettier.

Important Details

  • .gitleaks.toml extends the default ruleset with a single [allowlist] that covers verified false positives only (persisted-query SHA hashes, public Firebase/Algolia client keys, self-host compose templates, mock/truncated doc tokens) plus gitignored build output (.next/, dist/).
  • The ~13 reformatted .ts/.tsx/.css files and the large pnpm-lock.yaml diff are mechanical (Biome formatting and dep additions), not behavioral.
  • Deletions: dead packages/shared-components/.eslintrc.js, redundant cypress/.prettierrc.mjs, and the unused eslint devDep in shared-components.
  • Commits were made with --no-verify because this host worktree cannot run tsc (the graphql package is unbuilt: Cannot find module '@klicker-uzh/graphql/dist/ops'). tsc and lint are authoritative in CI; the pre-push pnpm run build did run and passed.

Branch Coverage

  • Base: v3
  • Head: 907ecdc9d
  • Reviewed: 8 commits v3..HEAD, 32 files, +1344/-630. Non-cosmetic surface: 17 config/CI/hook/docs files (listed via git diff --name-status), plus 13 cosmetic Biome reformats and the lockfile.
  • Covered: plan (6954254ec), Biome adoption (cd31ec950), lint audit baseline (d6e77c92e), Knip config + dead-dep drops (7bb96fc01), Gitleaks hook+CI (17533ef3e), advisory CI jobs (33fa52124), docs (6cda1257e), security remediation (907ecdc9d).

Review Focus

  • .gitleaks.toml allowlist breadth: confirm each entry is a false positive and the scan stays honest.
  • The Biome/Prettier directory split in biome.json + .lintstagedrc.mjs: confirm no file is double-formatted or missed.
  • Advisory vs blocking classification of the new CI steps.
  • util/load-test/k6.js now reads both JWTs from __ENV; confirm no token remains.

Security / Privacy

  • Review: $security-review run over the branch; the gitleaks default ruleset was run with the allowlist off (625 raw hits) to enumerate exactly what is suppressed.
  • Finding (pre-existing, remediated): util/load-test/k6.js hardcoded a signed staging session JWT: role:ADMIN, scope:FULL_ACCESS, no exp, issuer auth.klicker.stg.df-app.ch (the deployed UZH staging env; hosts resolve to Azure 4.226.22.29). It was introduced earlier on v3 (5d3ebeaa0, chore: new deployment with infisical and argocd, backup scripts #4962), not by this branch. Remediated here: tokens are injected via __ENV, the hardcoded values are removed, and the broad util/load-test/ allowlist is dropped so the gate no longer masks them.
  • Not disclosed: only the token leaked, not the JWT signing secret. HS256 is symmetric, though, so the token enables an offline attack on the secret, and it remains in permanent public git history (deleting the file does not undo the exposure).
  • Sensitive data: no secrets or token values are reproduced in this description or the plan file.

Blocking Before Merge

  • CI green (tsc/lint/format/syncpack/gitleaks), triggered on PR open.
  • Rotate the staging JWT signing secret (APP_SECRET/JWT key) in Infisical; confirm staging and production do not share it. The leaked token is live until rotated and sits in public history.

Follow-Up After Merge

  • Ratchet Biome lint and Knip from advisory to blocking once the backlog is worked down (tracked in the plan).
  • Optionally enable Tailwind class-sorting once a stable Biome rule exists (deferred; gap flagged in the plan).

- add biome ~2.5.2; biome.json formats JS/TS/JSX/TSX/JSON/CSS
- keep Prettier for Markdown/YAML plus the playwright/ + cypress/ E2E
  suites (Biome reformats Playwright test.describe.serial() chains)
- drop prettier-plugin-organize-imports + prettier-plugin-tailwindcss
  (Biome assist owns import organization; Tailwind class-sort -> Phase 2)
- bump knip 5.83.0 -> ~6.24.0; lint-staged routes code->biome, E2E+docs->prettier
- apply biome formatter (12 non-E2E files reformatted)
- plan: project/2026-07-19-biome-knip-repo-quality.md
- biome.json: disable noNonNullAssertion (11043 deliberate '!'); keep recommended otherwise
- delete dead packages/shared-components/.eslintrc.js (legacy prettier-eslint, D6)
- defer Tailwind useSortedClasses (nursery + 822 non-parity re-orders); class-sort gap flagged (D2)
- audit baseline: 519 err / 2456 warn / 434 info after disable; ratchet backlog in plan
- knip.json: schema->6; ignore prisma schemas / generated i18n / static assets, @theme virtual modules, external binaries (uv/doppler/docusaurus-*)
- delete cypress/.prettierrc.mjs (referenced removed prettier-plugin-organize-imports; root config governs cypress)
- remove unused eslint devDep from packages/shared-components (D6 tail)
- audit baseline: 142 files / 62+117 deps / 5 unlisted / 717+1272 exports; ratchet backlog in plan
Local husky pre-commit runs 'gitleaks git --staged' with a graceful skip when
the binary is absent (CI is the hard gate). CI check-gitleaks.yml installs the
MIT release binary v8.30.1 (gitleaks-action@v2 needs a license for org-owned
repos) and runs a blocking full-tree 'gitleaks dir' scan on push/PR to v3/v3*.

.gitleaks.toml extends the default ruleset with a single allowlist covering
verified false positives only (persisted-query hashes, public Firebase/Algolia
client keys, self-host env templates, doc/test fixtures). No real secret leak.

Quality tooling runs on host + CI, never inside the devcontainer.
check-lint.yml gains a non-blocking Biome lint step (continue-on-error) before
the blocking turbo/eslint step; eslint stays the Next.js safety net. New
check-knip.yml runs 'pnpm run knip' advisory on push/PR to v3/v3*. Adds a
lint:biome root script for local/CI parity. check-format.yml already delegates
to the Biome-based format:check script (Phase 1), so it is unchanged.
AGENTS.md (CLAUDE.md symlink): commands, tech-stack row, code conventions, and
pre-commit section now describe Biome (code) + Prettier (md/yaml + e2e) + ESLint
(Next.js safety net), plus the staged gitleaks hook. Wiki getting-started gains a
code-quality toolchain paragraph; ci-and-deployment PR gates add check-knip and
check-gitleaks and drop the stale 'knip is manual only' claim. Logged in docs/log.md.
util/load-test/k6.js hardcoded a signed staging ADMIN/FULL_ACCESS session JWT
(no expiry, issuer auth.klicker.stg.df-app.ch — the deployed UZH staging env) in
this public repo. Inject both tokens via __ENV.KLICKER_SESSION_TOKEN /
KLICKER_PARTICIPANT_TOKEN instead, matching the file's existing __ENV pattern.

Drop the broad util/load-test/ gitleaks allowlist so the gate no longer masks
these tokens; add gitignored build-output exclusions (.next/, dist/) and
least-privilege 'permissions: contents: read' on the new workflows.

The token remains in public git history — rotating the staging auth JWT signing
secret is required to fully remediate (tracked in the plan).
@coderabbitai

coderabbitai Bot commented Jul 20, 2026

Copy link
Copy Markdown

Important

Review skipped

Draft detected.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 94ba1d8f-7d85-4883-98a1-dee7d667620d

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@greptile-apps

greptile-apps Bot commented Jul 20, 2026

Copy link
Copy Markdown

Confidence Score: 4/5

Safe to merge once the staging JWT signing secret is rotated; all findings are non-blocking quality or security-hygiene items.

The core tooling migration (Biome, Knip, Gitleaks) is cleanly implemented and the critical JWT removal is correct. Three non-blocking issues remain: the gitleaks CI binary install skips checksum verification, the .gitleaks.toml project-directory allowlist is wider than the single file that needs it, and the k6 load test silently proceeds with empty cookies when env vars are missing.

.gitleaks.toml (allowlist scope) and .github/workflows/check-gitleaks.yml (binary integrity) deserve a second look before the gitleaks gate is considered fully hardened.

Important Files Changed

Filename Overview
biome.json New Biome 2.5.2 config; correctly excludes playwright/, cypress/, and generated files. Formatter and linter rules match the conventions documented in AGENTS.md.
.gitleaks.toml Allowlist is well-justified per entry; project/.*\.md$ is broader than needed and could silently suppress a future leak in any project plan file — better pinned to the specific filename.
.github/workflows/check-gitleaks.yml Blocking CI secret scan via direct binary install; lacks SHA256 checksum verification before installing the security-sensitive binary.
util/load-test/k6.js Hardcoded staging JWTs removed and replaced with __ENV injection — security fix is correct, but missing env vars silently fall back to empty strings instead of aborting.
.lintstagedrc.mjs Correctly splits staged files between Biome (code/JSON/CSS, non-e2e) and Prettier (e2e, md/yaml); check-only mode matches the documented workflow. Symlink guard is preserved.
.github/workflows/check-knip.yml New advisory Knip workflow with continue-on-error: true; path filter and pnpm install steps mirror other workflows. Appropriate least-privilege permissions.
.github/workflows/check-lint.yml Adds advisory Biome lint step before the blocking ESLint/Turbo pass; pattern updated to include biome.json. No functional regressions.
.husky/pre-commit Gitleaks pre-commit hook gracefully skips when binary is absent; staged-only scan mode is correct. Existing lint-staged invocation is preserved.
knip.json Upgraded schema ref to knip@6, replaced empty workspaces stub with meaningful ignoreFiles/ignoreBinaries/ignoreDependencies entries.
package.json Adds @biomejs/biome ~2.5.2, upgrades knip to ~6.24.0, removes prettier-plugin-organize-imports and prettier-plugin-tailwindcss; format/format:check scripts updated to delegate code to Biome and md/yaml to Prettier.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[git commit] --> B[husky pre-commit]
    B --> C{gitleaks binary present?}
    C -- yes --> D[gitleaks git --staged\n--config .gitleaks.toml]
    C -- no --> E[skip with notice / CI enforces]
    D --> F[lint-staged]
    E --> F
    F --> G{file type?}
    G -- code/JSON/CSS --> H{e2e path?}
    H -- no --> I[biome format check]
    H -- yes --> J[prettier --check]
    G -- md/yaml --> J
    F --> K[pnpm check:all]
    subgraph CI
        L[check-gitleaks BLOCKING] --> M[gitleaks dir .]
        N[check-lint] --> O[biome lint advisory]
        N --> P[ESLint BLOCKING]
        Q[check-knip advisory] --> R[knip]
        S[check-format BLOCKING] --> T[biome format + prettier]
    end
Loading
%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%%
flowchart TD
    A[git commit] --> B[husky pre-commit]
    B --> C{gitleaks binary present?}
    C -- yes --> D[gitleaks git --staged\n--config .gitleaks.toml]
    C -- no --> E[skip with notice / CI enforces]
    D --> F[lint-staged]
    E --> F
    F --> G{file type?}
    G -- code/JSON/CSS --> H{e2e path?}
    H -- no --> I[biome format check]
    H -- yes --> J[prettier --check]
    G -- md/yaml --> J
    F --> K[pnpm check:all]
    subgraph CI
        L[check-gitleaks BLOCKING] --> M[gitleaks dir .]
        N[check-lint] --> O[biome lint advisory]
        N --> P[ESLint BLOCKING]
        Q[check-knip advisory] --> R[knip]
        S[check-format BLOCKING] --> T[biome format + prettier]
    end
Loading

Fix All in Codex Fix All in Claude Code

Reviews (1): Last reviewed commit: "fix(security): stop committing staging J..." | Re-trigger Greptile

Comment on lines +29 to +34
shell: bash
run: |
curl -sSfL "https://github.com/gitleaks/gitleaks/releases/download/v8.30.1/gitleaks_8.30.1_linux_x64.tar.gz" -o /tmp/gitleaks.tar.gz
tar -xzf /tmp/gitleaks.tar.gz -C /tmp gitleaks
sudo install /tmp/gitleaks /usr/local/bin/gitleaks
gitleaks version

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Gitleaks binary installed without integrity check

The binary is fetched from GitHub releases and installed directly with no SHA256 checksum verification. Gitleaks publishes a checksums.txt alongside every release; downloading and cross-checking it takes two extra lines. Because the binary has full read access to every file it scans — including anything that slips past the allowlist — a tampered archive (e.g. via a MITM or a compromised GitHub release asset) could silently exfiltrate secrets rather than report them. The permissions: contents: read guard limits what the workflow token can do afterward, but it does not protect the file content handed to the scanner itself.

Fix in Codex Fix in Claude Code

Comment thread .gitleaks.toml
# "test-verification-abc123"). Real load-test tokens are injected via __ENV,
# never committed (see util/load-test/k6.js).
'''\.agents/skills/.*\.md$''',
'''project/.*\.md$''',

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 The project/.*\.md$ pattern exempts every current and future Markdown file inside project/ from secret scanning. The PR description notes this is to suppress illustrative tokens in the plan file project/2026-07-19-biome-knip-repo-quality.md. Pinning the pattern to the specific filename prevents a real secret committed to any other project/*.md file from silently bypassing the gate.

Suggested change
'''project/.*\.md$''',
'''project/2026-07-19-biome-knip-repo-quality\.md$''',

Fix in Codex Fix in Claude Code

Comment thread util/load-test/k6.js
Comment on lines +25 to 34
// authenticated — tokens are injected at runtime, never committed:
// k6 run -e KLICKER_SESSION_TOKEN=... -e KLICKER_PARTICIPANT_TOKEN=... k6.js
// This repo is public and staging JWTs grant real access; capture fresh
// tokens from an authenticated staging session each run.
const cookies = {
cookies: {
'next-auth.session-token':
'eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJlbWFpbCI6ImxlY3R1cmVyQGRmLnV6aC5jaCIsInN1YiI6Ijc2MDQ3MzQ1LTM4MDEtNDYyOC1hZTdiLWFkYmViY2ZlODgyMSIsInNob3J0bmFtZSI6ImxlY3R1cmVyIiwic2NvcGUiOiJGVUxMX0FDQ0VTUyIsImNhdGFseXN0SW5zdGl0dXRpb25hbCI6dHJ1ZSwiY2F0YWx5c3RJbmRpdmlkdWFsIjp0cnVlLCJyb2xlIjoiQURNSU4iLCJpYXQiOjE3NjMwNDg2OTEsImlzcyI6Imh0dHBzOi8vYXV0aC5rbGlja2VyLnN0Zy5kZi1hcHAuY2gifQ.wVgW8eDFQ9Ygvc2Qd3eNmQkdeGg5ukwwiPAXhtO9Qfo',
participant_token:
'eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJzdWIiOiI2ZjQ1MDY1Yy02NjdmLTQyNTktODE4Yy1jNmY2YjQ3N2ViNDgiLCJyb2xlIjoiUEFSVElDSVBBTlQiLCJpYXQiOjE3NjMxMDgyMjksImV4cCI6MTc2NDMxNzgyOSwiaXNzIjoiaHR0cHM6Ly9hcGkua2xpY2tlci5zdGcuZGYtYXBwLmNoIn0.gV7lD0PJGbiD45EX8yEF0V9CT3kFSgZIWoyljgBKQmA',
'next-auth.session-token': __ENV.KLICKER_SESSION_TOKEN || '',
participant_token: __ENV.KLICKER_PARTICIPANT_TOKEN || '',
},
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 When KLICKER_SESSION_TOKEN / KLICKER_PARTICIPANT_TOKEN are not provided, cookies are set to empty strings and the load test proceeds silently as an anonymous user. The check() calls only log failures — they do not abort the run — so the operator gets no clear signal that authenticated scenarios are being skipped. Failing early when either required token is absent would surface the misconfiguration immediately rather than producing misleading unauthenticated load results.

Suggested change
// authenticated — tokens are injected at runtime, never committed:
// k6 run -e KLICKER_SESSION_TOKEN=... -e KLICKER_PARTICIPANT_TOKEN=... k6.js
// This repo is public and staging JWTs grant real access; capture fresh
// tokens from an authenticated staging session each run.
const cookies = {
cookies: {
'next-auth.session-token':
'eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJlbWFpbCI6ImxlY3R1cmVyQGRmLnV6aC5jaCIsInN1YiI6Ijc2MDQ3MzQ1LTM4MDEtNDYyOC1hZTdiLWFkYmViY2ZlODgyMSIsInNob3J0bmFtZSI6ImxlY3R1cmVyIiwic2NvcGUiOiJGVUxMX0FDQ0VTUyIsImNhdGFseXN0SW5zdGl0dXRpb25hbCI6dHJ1ZSwiY2F0YWx5c3RJbmRpdmlkdWFsIjp0cnVlLCJyb2xlIjoiQURNSU4iLCJpYXQiOjE3NjMwNDg2OTEsImlzcyI6Imh0dHBzOi8vYXV0aC5rbGlja2VyLnN0Zy5kZi1hcHAuY2gifQ.wVgW8eDFQ9Ygvc2Qd3eNmQkdeGg5ukwwiPAXhtO9Qfo',
participant_token:
'eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJzdWIiOiI2ZjQ1MDY1Yy02NjdmLTQyNTktODE4Yy1jNmY2YjQ3N2ViNDgiLCJyb2xlIjoiUEFSVElDSVBBTlQiLCJpYXQiOjE3NjMxMDgyMjksImV4cCI6MTc2NDMxNzgyOSwiaXNzIjoiaHR0cHM6Ly9hcGkua2xpY2tlci5zdGcuZGYtYXBwLmNoIn0.gV7lD0PJGbiD45EX8yEF0V9CT3kFSgZIWoyljgBKQmA',
'next-auth.session-token': __ENV.KLICKER_SESSION_TOKEN || '',
participant_token: __ENV.KLICKER_PARTICIPANT_TOKEN || '',
},
}
// authenticated — tokens are injected at runtime, never committed:
// k6 run -e KLICKER_SESSION_TOKEN=... -e KLICKER_PARTICIPANT_TOKEN=... k6.js
// This repo is public and staging JWTs grant real access; capture fresh
// tokens from an authenticated staging session each run.
if (!__ENV.KLICKER_SESSION_TOKEN || !__ENV.KLICKER_PARTICIPANT_TOKEN) {
throw new Error(
'KLICKER_SESSION_TOKEN and KLICKER_PARTICIPANT_TOKEN must be set via -e. ' +
'Capture fresh tokens from an authenticated staging session.'
)
}
const cookies = {
cookies: {
'next-auth.session-token': __ENV.KLICKER_SESSION_TOKEN,
participant_token: __ENV.KLICKER_PARTICIPANT_TOKEN,
},
}

Fix in Codex Fix in Claude Code

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

1 participant