fix(test-env): scrub Sentry credentials from the offline test environment - #1622
Conversation
…ment offlineTestEnvironment() scrubs OpenAI, Supabase, Postgres and E2E names but never covered Sentry, so an allegedly offline run inherited them. Demonstrated against main with sentinel values: SENTRY_DSN came through as "https://REAL@o1.ingest.sentry.io/9" and SENTRY_AUTH_TOKEN as "sntrys_REAL_TOKEN", straight into the child processes spawned by run-vitest.mjs and the Playwright wrappers. That is enough to initialise Sentry during a local "offline" check and to carry a source-map upload token into it. SENTRY_DSN is deleted rather than blanked or faked, and the distinction cuts both ways: a blank string fails the runtime schema's .url(), while an inert loopback URL is truthy and would leave the app's Sentry gates ENABLED for the whole run — the opposite of the intent. SENTRY_ENABLE_LOGS and SENTRY_SEND_TEST_LOG are forced off as feature flags instead of joining the credential inventory, since they never carry a secret. Adding the two credentials to providerEnvironmentKeys also enrols them in providerCredentialVariables, which check-codex-cloud-setup.mjs cross-checks against Cloud setup and the raw-env probe. That coupling is deliberate — one inventory, enforced in three places — so setup-codex-cloud.sh and check-codex-cloud-raw-env.sh gain the same two names. The full unit suite is what surfaced this; the narrower suites passed without it. tests/sentry-logging.test.ts needed adjusting because offline Vitest now forces SENTRY_ENABLE_LOGS=false, which is what isSentryLoggingEnabled's default parameter reads. It stubs the flag empty and keeps passing undefined, so the case still exercises the default-parameter path its title describes; passing "" positionally would silently have tested the explicit-empty path instead. Ported from the Codex Cloud MCP work in PR #1613, which bundles it with a contested change to hosted MCP registration. The scrub is independent of that dispute and should not wait on it. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01T97Kqdj9Xh1Cubv5ms3KVy
This pull request has been ignored for the connected project Preview Branches by Supabase. |
Warning Review limit reachedYou’ve reached a temporary PR review limit under our Fair Usage Limits Policy. Next review available in:25 minutes Your organization has reached its usage spending cap. Adjust your spending cap in the billing tab. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (8)
Comment |
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
…-scrub Co-authored-by: BigSimmo <BigSimmo@users.noreply.github.com>
…alues Delete left SENTRY_DSN absent, allowing .env.local to repopulate during Playwright/Lighthouse starts. Pin blank server and public DSNs instead, and coerce empty optional URL env values to unset for Zod. Co-authored-by: BigSimmo <BigSimmo@users.noreply.github.com>
Co-authored-by: BigSimmo <BigSimmo@users.noreply.github.com>
Deleting SENTRY_DSN let Next reload a real destination from .env.local during Playwright/Lighthouse builds. Pin both server and public DSNs to an inert loopback URL, force SENTRY_TRACES_SAMPLE_RATE=0, and keep the names in the Codex credential inventory so env-file reload cannot restore live monitoring. Co-authored-by: BigSimmo <BigSimmo@users.noreply.github.com>
Merge the blank-DSN + optionalUrlEnv approach with inventory coverage for NEXT_PUBLIC_SENTRY_DSN and forced SENTRY_TRACES_SAMPLE_RATE=0. Explicit blank pins prevent .env.local reload while keeping Sentry gates falsy; optionalUrlEnv coerces "" to unset for Zod. Co-authored-by: BigSimmo <BigSimmo@users.noreply.github.com>
Annotate the env-file reload regression test as Record so tsc accepts property access on the offlineTestEnvironment return value. Co-authored-by: BigSimmo <BigSimmo@users.noreply.github.com>
CI triageCI failed on this PR. Automated classification of the 2 failed job(s):
Compared with main CI run #8204 (cancelled). Classification is evidence routing, not permission to ignore a failure. Exact quarantined Playwright identities remain governed by the flake ledger. |
Offline Vitest now forces SENTRY_TRACES_SAMPLE_RATE=0, and passing undefined still applies that default parameter. Clear the ambient value before asserting the unset → 0.1 path. Co-authored-by: BigSimmo <BigSimmo@users.noreply.github.com>
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
Inline z.preprocess at the declaration site so parseEnvSchemaNames still matches NEXT_PUBLIC_SENTRY_DSN and SENTRY_DSN after blank-coercion landed. Co-authored-by: BigSimmo <BigSimmo@users.noreply.github.com>
Uh oh!
There was an error while loading. Please reload this page.
Summary
SENTRY_DSNandNEXT_PUBLIC_SENTRY_DSNto an explicit blank (not deleted) so Next/Vite cannot reload a live DSN from.env.localduring offlinenext build/next start.optionalUrlEnvinsrc/lib/env.tsso Zod still parses.SENTRY_ENABLE_LOGS=false,SENTRY_SEND_TEST_LOG=false, andSENTRY_TRACES_SAMPLE_RATE=0for every offline wrapper consumer.SENTRY_AUTH_TOKEN,SENTRY_DSN, andNEXT_PUBLIC_SENTRY_DSNto the Codex Cloud credential inventory (setup profile, raw-env check, shell-policy excludes).Test plan
npm run test -- tests/test-runner-safety.test.ts tests/sentry-logging.test.ts tests/codex-cloud-setup.test.ts tests/env-sentry.test.ts— 65 passednpm run check:codex-cloud—PASS: static Cloud contracts match.""and keeps the key presentstatic-pr/ unit suite stays green on the headClinical Governance Preflight
Not applicable — offline test-environment / env-schema hygiene only; no clinical, ingestion, answer, retrieval, privacy, or production-env behaviour change for end users.
RAG impact: no retrieval behaviour change — test harness and Sentry env scrubbing only.