fix(web): validate signalId as a CUID in signals server functions - #3912
Draft
geclos wants to merge 1 commit into
Draft
fix(web): validate signalId as a CUID in signals server functions#3912geclos wants to merge 1 commit into
geclos wants to merge 1 commit into
Conversation
Every signals.functions.ts input schema accepted signalId as a bare z.string(), then cast it with the unchecked SignalId() helper before passing it straight into ClickHouse-backed repository calls (several of which query columns typed FixedString(24)). A malformed signalId (e.g. a stale deep-link query param) skipped the signalIdSchema (CUID) validation already used everywhere else in the domain layer and reached the ClickHouse driver raw, surfacing as an unhandled RepositoryError instead of a clean validation error. Switch every signalId/signalIds field in this file to the existing signalIdSchema (cuidSchema.transform(SignalId)), matching the convention already used in packages/domain/signals use-cases. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01AmwWE7kpvpNYbbmRzEJVDd
|
The latest updates on your projects. Learn more about Vercel for GitHub. 1 Skipped Deployment
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What does this PR do?
Fixes a production crash surfaced in Datadog Error Tracking:
596d8076-7a0c-11f1-8afa-da7ad0900005—RepositoryError: Too large value for FixedString(24): value structured-output-json-parse-failure cannot be parsed as FixedString(24) for query parameter 'signalId'inweb(GET,listSignalSessions).Root cause: every input schema in
apps/web/src/domains/signals/signals.functions.ts(~13 of them) validatedsignalId/signalIdsas a barez.string(), then cast it through the uncheckedSignalId()helper ((value: string): SignalId => value as SignalId— a type-cast, not a validator) before passing it straight intoScoreAnalyticsRepository/ScoreRepositorycalls. Several of those repository methods (listSessionsBySignal,countSessionsBySignal,listTracesBySignal,countTracesBySignal,trendBySignal) query ClickHouse columns typedFixedString(24). Any malformedsignalId— e.g. a stale/bad deep-link query param — skipped thesignalIdSchema(CUID) validation already used everywhere else in the domain layer (packages/domain/signals/src/use-cases/{update-signal,delete-signal,apply-signal-lifecycle-command,list-signals}.ts) and reached the ClickHouse driver raw, throwing an unhandledRepositoryErrorinstead of a clean 400.This wasn't a one-off: the same unchecked
z.string()→SignalId()pattern appeared at every sibling call site in the file (getSignalRowMetrics,listSignalSessions,countSignalSessions,getSignalImpact,getSignalDimensions,getRelatedSignals,getSignalOccurrences,updateSignalTriage,applySignalLifecycleAction,updateSignal,updateSignalEvaluation,deleteSignal), so the fix replacesz.string()/z.array(z.string())with the existingsignalIdSchema/z.array(signalIdSchema)(cuidSchema.transform(SignalId)) across all of them, matching the convention already established in the domain layer. No behavior changes for well-formed IDs; malformed ones now fail fast with a validation error at the server-function boundary instead of a raw infra exception downstream.Related issue (if applicable)
Datadog Error Tracking issue: https://app.datadoghq.eu/error-tracking/issue/596d8076-7a0c-11f1-8afa-da7ad0900005 (first seen 2026-07-07, low volume so far — new regression, not yet an open GitHub issue)
Closes #
How was this tested?
apps/web/src/domains/signals/signals.functions.test.ts, exercisingsignalTracesInputSchema(used by the crashinglistSignalSessionsfunction): accepts a well-formed CUID, rejects the exact malformed value from the Datadog error (structured-output-json-parse-failure) and an empty string.signalIdtoz.string()) and pass with the fix restored.pnpm --filter web typecheck(tsgo) — clean.pnpm exec biome checkon changed files — clean.pnpm --filter web exec vitest run src/domains/signals— all 13 tests pass, no regressions in the existing signals test suite.Verification in production: after deploy, occurrences of Datadog issue
596d8076-7a0c-11f1-8afa-da7ad0900005should stop; any future malformedsignalIdwill surface as a clean 400 validation error rather than an unhandledRepositoryError.Checklist
Generated by Claude Code