Skip to content

chore(evals): vitest-evals Select suite + release-gated CI - #729

Draft
mattrothenberg wants to merge 7 commits into
mainfrom
feat/vitest-evals
Draft

chore(evals): vitest-evals Select suite + release-gated CI#729
mattrothenberg wants to merge 7 commits into
mainfrom
feat/vitest-evals

Conversation

@mattrothenberg

Copy link
Copy Markdown
Collaborator

Summary

Replaces the promptfoo-based Select evals with vitest-evals. Adds an eval harness, deterministic assertions, a public-context loader, an 8-case Select suite, and a release-gated CI job that runs the evals against the "Version Packages" PR and posts a summary comment.

What's included

  • Harness (evals/harness/cloudflare-ai.ts): calls Cloudflare AI Gateway (same org secrets Bonk uses) with a direct Workers AI fallback for local dev.
  • Assertions (evals/assertions/select.ts): parses generated TSX with @babel/parser and validates the Select API via AST. Parse-only — generated model code is never executed.
  • Context (evals/context/select.ts): builds prompt context from public sources only (shipped registry JSON, public docs .mdx, public demo). No private data.
  • Suite (evals/select.eval.ts) + vitest.evals.config.ts.
  • Scripts: evals, evals:select, evals:ci, evals:ui.
  • CI (.github/workflows/release.yml): new evals job, if: github.repository_owner == 'cloudflare', environment: kumo-evals, least-privilege perms (contents: read, pull-requests: write). Uploads results artifact (30d) and comments pass/fail on the Version Packages PR.

Safety notes

  • Triggers only on push: main (never pull_request/pull_request_target) — forks cannot reach the AI Gateway secrets.
  • Generated code is AST-parsed, not run.
  • evals/.generated/ is gitignored; result JSON verified to contain no secrets.
  • Eval files typecheck clean locally. They live at repo root outside any package, so CI test/typecheck never collect them — the eval suite will not red-X normal PR CI.

Open decisions for review (why this is a draft)

  1. Trigger model. As wired, the evals job auto-runs (behind environment approval) on every push to main while a Version Packages PR is open — evals are slow/expensive, so this may be too noisy. Proposed alternative: switch to workflow_dispatch (manual) + kumo-evals approval so it runs deliberately pre-release. Feedback wanted.
  2. Environment protection (required before merge). kumo-evals must have required reviewers configured, and CF_AI_GATEWAY_* should be environment secrets (not repo-wide) so the approval gate actually protects them.
  3. Token scope. Confirm the AI Gateway token is inference-only and scoped to the eval model; consider a dedicated token separate from Bonk's for independent rotation.

Testing

Local eval run passes 8/8. tsc --noEmit on all four eval files passes.

  • Reviews
  • automated review not possible because: infra/CI change requiring human decisions on secret scoping and environment protection
  • Tests
  • Additional testing not necessary because: eval suite is the test artifact; it runs 8/8 locally and is excluded from normal CI by design

Replace promptfoo-based Select evals with vitest-evals:
- Eval harness (Cloudflare AI Gateway, direct Workers AI fallback)
- Deterministic AST assertions (parse-only, no code execution)
- Public-context loader (docs + generated registry)
- Select eval suite (8 cases) + vitest.evals.config.ts
- Root scripts: evals, evals:select, evals:ci, evals:ui
- Release workflow job gated to cloudflare org + kumo-evals environment,
  runs on Version Packages PR and posts a summary comment
@mattrothenberg

Copy link
Copy Markdown
Collaborator Author

/bonk can you please do an initial but thorough security review of this?

@ask-bonk

ask-bonk Bot commented Aug 12, 2026

Copy link
Copy Markdown
Contributor

Security Review: PR #729

Overall Assessment: Generally well-designed with good security fundamentals, but 2–3 items need attention before merge.


✅ What's done well

Practice Observed
No pull_request_target Trigger is push: main only — forks cannot reach secrets via PRs
Owner gating github.repository_owner == 'cloudflare' on both jobs
Environment protection environment: kumo-evals isolates secrets behind an approval gate
Least-privilege permissions contents: read + pull-requests: write for evals job
Parse-only execution model Generated code is AST-inspected by @babel/parser; eval(), new Function(), and dynamic import() are absent
No hardcoded credentials All secrets are env-var sourced
Minimal checkout fetch-depth: 1 for evals job limits CI surface
Gitignored results evals/.generated/ excluded from version control

⚠️ Medium concern: Error messages may leak gateway details

File: evals/harness/cloudflare-ai.ts (lines 83–86)

throw new Error(
  `Workers AI request failed: ${response.status} ${JSON.stringify(body.errors)}`,
);

If the AI Gateway or Workers AI returns an error containing internal routing info, partial token identifiers, or account metadata, it will be written to CI logs. These logs are visible to anyone with repo read access.

Recommendation: Redact or gate the raw error body. Log response.status unconditionally, but only include JSON.stringify(body.errors) when a debug flag is set.

const debug = process.env.KUMO_EVAL_DEBUG === "1";
const detail = debug ? ` ${JSON.stringify(body.errors)}` : "";
throw new Error(`Workers AI request failed: ${response.status}${detail}`);

⚠️ Medium concern: kumo-evals environment must be configured before merge

The environment: kumo-evals gate is only as strong as the GitHub environment settings. If required reviewers are not configured on that environment, the job runs automatically and the org secrets are immediately accessible to any main branch commit (including maliciously introduced workflow changes in future PRs).

Recommendation: This is already flagged in the PR description, but I want to emphasize it — please confirm the environment has required reviewers and that the three CF_AI_GATEWAY_* secrets are scoped only to this environment (not repo-level) before merging.


⚠️ Low concern: Unescaped URL interpolation

File: evals/harness/cloudflare-ai.ts (lines 60, 63)

url = `https://gateway.ai.cloudflare.com/v1/${gatewayAccountId}/${gatewayId}/workers-ai/${model}`;

KUMO_EVAL_MODEL is user-controllable via env. If it contains characters like ?, #, or path traversal sequences, the request URL is altered. Inbound SSRF is not possible here (it's an outbound call), but it could cause requests to hit unexpected gateway endpoints under the same origin.

Recommendation: Encode the model segment:

const encodedModel = encodeURIComponent(model);
url = `https://gateway.ai.cloudflare.com/v1/${gatewayAccountId}/${gatewayId}/workers-ai/${encodedModel}`;

ℹ️ Informational

  1. Dependency: vitest-evals — This is a relatively new package (v0.16.1). Standard supply-chain diligence applies; pinning to an exact version in pnpm-lock.yaml is already done.
  2. Local DX: --env-file=.env.local — On local runs, if .env.local is absent the script exits immediately. Not a security issue, but you may want a graceful fallback with a helpful error message.
  3. CI artifact — 30-day retention on vitest-results.json is fine given the author's verification that it contains no prompt text or model responses.

Summary

The architecture is solid: secrets are behind an environment gate, code generation is parse-only, and the trigger model correctly avoids pull_request_target exposure.

Pre-merge blockers:

  1. Confirm kumo-evals environment has required reviewers enabled and the three CF_AI_GATEWAY_* values are environment-scoped secrets.
  2. Consider sanitizing the body.errors output in cloudflare-ai.ts before it hits CI logs.

Nice-to-have: URL-encode the model path segment.

LGTM once environment protection is confirmed.

github run

@pkg-pr-new

pkg-pr-new Bot commented Aug 12, 2026

Copy link
Copy Markdown
npm i https://pkg.pr.new/@cloudflare/kumo@729

commit: 105bb57

@github-actions

github-actions Bot commented Aug 12, 2026

Copy link
Copy Markdown
Contributor

Docs Preview

View docs preview

Commit: 7d95ac4

- Gate raw upstream error body behind KUMO_EVAL_DEBUG to avoid leaking
  gateway/account metadata into CI logs
- Encode model path per-segment (preserving '/' separators) to neutralize
  '?'/'#'/traversal chars in a KUMO_EVAL_MODEL override
@github-actions

github-actions Bot commented Aug 13, 2026

Copy link
Copy Markdown
Contributor
Visual Regression Report — 24 changed, 19 unchanged

24 screenshot(s) with visual changes:

Button / Basic

243 px (0.27%) changed

Before After Diff
Before After Diff

Button / With Icon

797 px (0.9%) changed

Before After Diff
Before After Diff

Button / Loading State

1,121 px (1.27%) changed

Before After Diff
Before After Diff

Button / Disabled State

0 px (0%) changed

Before After Diff
Before After Diff

Button / Title

405 px (0.46%) changed

Before After Diff
Before After Diff

Button / Link as Button

1,204 px (1.36%) changed

Before After Diff
Before After Diff

Button / Link with Tooltip

721 px (0.81%) changed

Before After Diff
Before After Diff

Dialog / Dialog With Actions

160 px (0.18%) changed

Before After Diff
Before After Diff

Dialog / Dialog Basic

403 px (0.46%) changed

Before After Diff
Before After Diff

Dialog / Dialog Sizes

2,936 px (3.32%) changed

Before After Diff
Before After Diff

Dialog / Dialog Alert

1,885 px (2.13%) changed

Before After Diff
Before After Diff

Dialog / Dialog With Dropdown

494 px (0.56%) changed

Before After Diff
Before After Diff

Dialog (Open)

0 px (0%) changed

Before After Diff
Before After Diff

Select / Select Without Label

746 px (0.84%) changed

Before After Diff
Before After Diff

Select / Select With Description

1,688 px (1.62%) changed

Before After Diff
Before After Diff

Select / Select With Error

2,126 px (2.04%) changed

Before After Diff
Before After Diff

Select / Select Placeholder

1,328 px (1.5%) changed

Before After Diff
Before After Diff

Select / Select Custom Rendering

1,153 px (1.3%) changed

Before After Diff
Before After Diff

Select / Select Loading

0 px (0%) changed

Before After Diff
Before After Diff

Select / Select Multiple

1,630 px (1.84%) changed

Before After Diff
Before After Diff

Select / Select Complex

1,655 px (1.59%) changed

Before After Diff
Before After Diff

Select / Select Grouped

105 px (0.12%) changed

Before After Diff
Before After Diff

Select / Select Grouped With Disabled

1,189 px (1.34%) changed

Before After Diff
Before After Diff

Select (Open)

2,673 px (0.01%) changed

Before After Diff
Before After Diff
19 screenshot(s) unchanged
  • Button / Variant: Primary
  • Button / Variant: Secondary
  • Button / Variant: Ghost
  • Button / Variant: Destructive
  • Button / Variant: Outline
  • Button / Variant: Secondary Destructive
  • Button / Sizes
  • Button / Icon Only
  • Button / Disabled Link
  • Dialog / Dialog Confirmation
  • Dialog / Custom Max Width
  • Dialog / Dialog With Select
  • Dialog / Dialog With Combobox
  • Select / Select Basic
  • Select / Select Sizes
  • Select / Select With Tooltip
  • Select / Select Disabled Options
  • Select / Select Disabled Items
  • Select / Select Long List

Generated by Kumo Visual Regression

@mattrothenberg

Copy link
Copy Markdown
Collaborator Author

Thanks @ask-bonk — addressed the two code items in f9c553c:

1. Error-body leak (medium) — ✅ Fixed. Raw body.errors is now gated behind KUMO_EVAL_DEBUG=1; the default failure message only includes response.status, so no upstream routing/account metadata hits CI logs.

2. Model URL interpolation (low) — ✅ Fixed, but with a correction. The suggested encodeURIComponent(model) would actually break routing: model IDs are multi-segment paths (@cf/moonshotai/kimi-k2.7-code) and blanket-encoding turns the / separators into %2F. I encode each segment instead, preserving the slashes while neutralizing ?/#/traversal chars in any override:

const encodedModelPath = model.split("/").map(encodeURIComponent).join("/");

3. kumo-evals environment (medium / pre-merge blocker) — Acknowledged, tracked as a pre-merge blocker in the PR description. This is a GitHub settings change (required reviewers + environment-scoped CF_AI_GATEWAY_* secrets), not code — will confirm before this leaves draft.

Informational items (new vitest-evals dep, .env.local DX, artifact retention) noted; no action needed for now.

@mattrothenberg

Copy link
Copy Markdown
Collaborator Author

/bonk re-review

@ask-bonk

ask-bonk Bot commented Aug 13, 2026

Copy link
Copy Markdown
Contributor

Re-review: PR #729 — Security

Status: LGTM (modulo the pre-merge environment settings blocker).


✅ Previous findings — verified fixed

Issue File Status
Error-body leak evals/harness/cloudflare-ai.ts:91-95 ✅ Gated behind KUMO_EVAL_DEBUG=1. Default CI logs only show response.status.
Model URL interpolation evals/harness/cloudflare-ai.ts:46,64,67 ✅ Per-segment encodeURIComponent correctly preserves multi-segment model paths (e.g. @cf/moonshotai/kimi-k2.7-code) while neutralizing ?, #, and traversal.

🔍 Fresh review — no new security blockers

Architecture checks

  • Trigger model: push: main only, owner-gated, no pull_request_target. Correct.
  • Secrets scope: CF_AI_GATEWAY_* are workflow env vars on the kumo-evals environment job. They are not repo-wide or passed into the release job. Correct.
  • Permissions: contents: read + pull-requests: write on the evals job. No id-token: write. Correct.
  • Parse-only: Assertions use @babel/parser AST inspection. No eval, new Function, or dynamic import. Correct.
  • Public context only: buildSelectContext() reads shipped registry JSON, public docs .mdx, and public demo files. Correct.
  • Artifact safety: evals/.generated/ is gitignored. The JSON-reported failures contain only assertion reason strings (e.g. "missing placeholder") and test titles from hardcoded eval cases — no raw model output or prompts. Correct.

Workflow review

  • needs.release.outputs.pullRequestNumber is used only to gate the evals job and post a summary comment. No secrets flow into actions/github-script inputs. The default GITHUB_TOKEN is scoped only to this repository.

Dependency

  • vitest-evals@^0.16.1 is supply-chain pinned by pnpm-lock.yaml. (Minor: consider exact-version in package.json to prevent accidental bumps on pnpm update.)

⚠️ Pre-merge blocker (non-code)

The kumo-evals GitHub environment must still be configured with required reviewers and the three CF_AI_GATEWAY_* secrets must be environment-scoped before this merges. This is already tracked in the PR description.


💡 Optional recommendation

You flagged this yourself in the PR description — switching the evals job trigger from automatic push: main to workflow_dispatch with the kumo-evals environment approval would make the runs deliberate rather than noisy. It also slightly reduces attack surface (fewer automatic executions of a secret-bearing workflow). Not a blocker, but worth considering as a follow-up.

github run

@mattrothenberg

Copy link
Copy Markdown
Collaborator Author

/bonk one more review please

@ask-bonk

ask-bonk Bot commented Aug 13, 2026

Copy link
Copy Markdown
Contributor

Security Review: PR #729 — Third Pass

Status: Changes requested — 1 critical, 1 medium


🔴 Critical: Missing environment declaration in evals.yml

The evals.yml workflow consumes CF_AI_GATEWAY_* secrets but the evals job does not declare environment: kumo-evals. This is a significant departure from the approval-gated design discussed in prior reviews. Without the environment declaration:

  • If the secrets are repo-scoped: any repository collaborator with workflow dispatch permissions can trigger this workflow and obtain the AI Gateway credentials.
  • If the secrets are environment-scoped: the workflow will fail at runtime because the job never requests the environment context.

Fix: Add the environment to the job:

jobs:
  evals:
    environment: kumo-evals
    # ... existing config

This must land before merge and be paired with:

  1. kumo-evals environment configured with required reviewers.
  2. CF_AI_GATEWAY_ACCOUNT_ID, CF_AI_GATEWAY_NAME, and CF_AI_GATEWAY_TOKEN stored as environment-scoped secrets (not repo-wide).

🟡 Medium: vitest-evals version range

package.json declares "vitest-evals": "^0.16.1". While pnpm-lock.yaml pins the exact version, the caret allows minor bumps on pnpm update. For a relatively new package, exact-version pinning in package.json (e.g., "0.16.1") provides defense in depth against unexpected upstream changes.


✅ Verified (unchanged since last review)

Control Status
workflow_dispatch only (no automatic triggers) ✅ Correct
Owner gating (github.repository_owner == 'cloudflare') ✅ Correct
Least-privilege permissions (contents: read, pull-requests: write) ✅ Correct
PR validation (branch, title, file list, source repo) ✅ Thorough
Error-body redaction (gated behind KUMO_EVAL_DEBUG=1) ✅ Correct
URL per-segment encoding ✅ Correct
Parse-only assertions (no eval, new Function, dynamic import) ✅ Confirmed
Public context sources only ✅ Confirmed
evals/.generated/ gitignored ✅ Confirmed
Artifact comment does not leak raw model output ✅ Failure messages are hardcoded assertion reasons

ℹ️ Minor notes

  • The PR description references .github/workflows/release.yml for the evals job, but the actual change is a new standalone .github/workflows/evals.yml. Updating the description avoids confusion.
  • The evals:ci script correctly omits --env-file=.env.local, ensuring CI does not depend on a local file.
  • concurrency group is github.workflow + github.ref, which for workflow_dispatch defaults to the branch the workflow runs on. This is fine.

Pre-merge blockers

  1. BLOCKER: Add environment: kumo-evals to the evals job.
  2. BLOCKER: Configure kumo-evals environment with required reviewers.
  3. BLOCKER: Move CF_AI_GATEWAY_* secrets to environment scope.
  4. Recommended: Pin vitest-evals to exact version in package.json.

Please re-request review once the environment declaration is added.

github run

@mattrothenberg

Copy link
Copy Markdown
Collaborator Author

/bonk updated

@ask-bonk

ask-bonk Bot commented Aug 13, 2026

Copy link
Copy Markdown
Contributor

Security Re-review: PR #729

Status: LGTM. All code-level blockers resolved.


✅ Previous blockers — verified fixed

Issue Status
Missing environment: kumo-evals .github/workflows/evals.yml:20 now declares environment: kumo-evals.
vitest-evals version range package.json:50 now pins "vitest-evals": "0.16.1" (exact version, no caret).

🔍 Architecture re-check — no regressions

Workflow (evals.yml)

  • Trigger: workflow_dispatch only. No automatic triggers. ✅
  • Owner gate: if: github.repository_owner == 'cloudflare'. ✅
  • Environment gate: environment: kumo-evals added. ✅
  • Least-privilege permissions: contents: read + pull-requests: write. ✅
  • PR validation: extensive checks on branch, title, file list, and source repo. ✅
  • Minimal checkout: fetch-depth: 1. ✅
  • Timeout: timeout-minutes: 20. ✅

Secrets

  • CF_AI_GATEWAY_* are workflow env vars scoped only to the evals job. Not passed to other jobs. ✅

Execution model

  • Generated code parsed via @babel/parser AST inspection only. No eval, new Function, or dynamic import. ✅

Context & artifacts

  • Context built from shipped registry JSON, public docs .mdx, public demos. No private data. ✅
  • evals/.generated/ is gitignored. ✅
  • Artifact comment only exposes test titles and hardcoded assertion reasons. ✅

ℹ️ Remaining pre-merge items (GitHub settings, not code)

The environment gate now works in the workflow, but the repository admin must still ensure the below are configured before the first run succeeds. These are not additional blockers for this PR — they are the same infrastructure tasks already flagged in the PR description.

Setting Why
kumo-evals environment has required reviewers Enforces approval gate before secrets are accessible.
CF_AI_GATEWAY_ACCOUNT_ID, CF_AI_GATEWAY_NAME, CF_AI_GATEWAY_TOKEN are environment-scoped secrets Ensures they are only released to the kumo-evals context.

Summary

The critical code gaps from the last review (missing environment declaration, loose version range) are closed. The design is sound: secrets are behind an approval gate, generated code is parse-only, and the trigger model correctly avoids pull_request_target exposure.

Approved for merge once the kumo-evals environment protection and secret scoping are confirmed by a repo admin.

github run

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant