Repository navigation
fix(worker-utils): validate the form of accepted schema patterns - #9322
Conversation
A webhook body schema's `pattern` keyword is now checked against the documented accepted subset when a trigger is created or updated: backreferences, quantifiers nested beyond the 64-repetition budget, and a repeated alternation whose branches are not disjoint are refused with a message that names the rule. The schema byte, depth and node limits are unchanged. `parseStoredWebhookBodySchema` reads a stored schema back with the compile check only, so a trigger configured earlier still loads, and `validateWebhookBody` leaves out a pattern the write path refuses while the capture path records that the stored schema was not applied.
iscekic
added
drain
![kilo-code-bot[bot]](https://avatars.githubusercontent.com/in/2193792?s=60&v=4){kind=small}
Code Review SummaryStatus: No Issues Found | Recommendation: Merge Executive SummaryThe two-character literal-prefix refinement resolves the carried first-character false refusal: Assumptions: Files Reviewed (2 files, incremental pass)
Previous Review Summaries (3 snapshots, latest commit f9d8d0e)Current summary above is authoritative. Previous snapshots are kept for context only. Previous review (commit f9d8d0e)Status: 1 Issue Found | Recommendation: Address before merge Executive SummaryCommit Assumptions: Overview
Issue Details (click to expand)WARNING
Files Reviewed (6 files in incremental pass)
Previous review (commit 41ecc5b)Status: 5 Issues Found | Recommendation: Address before merge Executive SummaryIncremental commit Assumptions: Overview
Issue Details (click to expand)WARNING
SUGGESTION
Files Reviewed (3 files in incremental pass)
Previous review (commit 147bac8)Status: 5 Issues Found | Recommendation: Address before merge Executive SummaryThe pattern-safety gate has one material hole: ambiguity is only checked when the alternation is the repeat's direct body, so Assumptions: Overview
Issue Details (click to expand)WARNING
SUGGESTION
Files Reviewed (8 files)
Reviewed by deepseek-flash-latest · Input: 56.6K · Output: 9.4K · Cached: 170.5K Review guidance: REVIEW.md from base branch |
iscekic
added
merge:blocked:kilobot-not-ready
The trpc project typechecks packages/worker-utils with noPropertyAccessFromIndexSignature, so the schema walker reads `pattern` and `properties` through the index signature.
iscekic
added
merge:blocked:kilobot-not-ready
- The repeated-alternation check walks alternatives at every depth of the repeated body, not only at its top level. - Literals and character-class ranges are read as code points, so an astral character stays one atom. - A stored schema whose pattern is outside the accepted subset is not applied at all instead of having single keywords removed, because removing a keyword changes what a `not`, `anyOf` or `oneOf` parent means. The validation result reports those schema locations, so the capture path logs them from the same walk that makes the decision.
iscekic
added
merge:blocked:kilobot-not-ready
…aracter Two alternatives that begin with the same character are still unambiguously ordered when both must consume a second character and those second characters differ, so the repeated-alternation rule now compares a literal prefix of up to two characters before refusing an alternation.
iscekic
added
merge:blocked:kilobot-not-ready
Evidence
Evidence: demonstrated — ran the real ingest Worker and its Durable Object in workerd on this VM (vitest workers pool) and over HTTP through
SELF.fetch. A trigger whose body schema carries a pattern outside the accepted subset is refused at create and update with the rule's message; a trigger whose schema is inside the subset validates deliveries as before, and a schema read back from storage still loads throughgetConfigandgetConfigForResponse. The web form has no interactive test harness inapps/web, so its message is proven from the shared schema (see below), not in a browser.Change
packages/worker-utils/src/webhook-body-validation.tsnow checks a schema'spatternkeyword against the documented accepted subset at write time:packages/worker-utils/src/pattern-safety.tsparses a pattern with the unicode grammar the validator uses and reports the first rule of the subset it breaks; the message names that rule.^issue\.(opened|closed)$,(\d{1,3}\.){3}\d{1,3},^(a{1,3}){1,3}$,(?:foo|bar)+,^(?:cat|cow)+$, lookarounds, named groups, astral characters and the existing schema example; alternatives that begin alike stay accepted when a second character separates them.validateWebhookBodyare unchanged.parseStoredWebhookBodySchemaparses a schema read from storage with the compile check only, so a trigger configured earlier still loads and keeps capturing; the capture path records when a stored schema is skipped.apps/web/src/components/webhook-triggers/BodyValidationField.tsxvalidates the textarea with the samewebhookBodySchemaand rendersissue.message, so the refusal appears in the form without a change:properties.event.pattern: Pattern must not nest quantifiers that exceed 64 combined repetitions(produced by a realsafeParseof that schema).Verification
pnpm --filter @kilocode/worker-utils vitest runpnpm --filter cloudflare-webhook-agent-ingest vitest runpnpm --filter cloudflare-webhook-agent-ingest vitest run --config vitest.workers.config.tspnpm --filter @kilocode/trpc run typecheckbash scripts/typecheck-all.sh --changes-onlyoxlinton both changed source trees,oxfmt --list-different,git diff --checkNew coverage:
pattern-safety.test.ts: the accepted and refused forms of the subset.webhook-body-validation.test.ts: the message and path of each refusal; a schema read back from storage keeps its content and reporting.trigger-do.test.ts: create and update refuse the pattern; a schema written directly into the trigger row under the previous policy loads and captures.routes.test.ts: the same stored schema delivers over the ingest HTTP route with the test's time bound.api.test.ts: the internal API input schemas refuse the pattern and report the field path.Production prerequisites
None.
Proxy answers
Answer: Drop the refused pattern and keep delivering
Confidence: 0.62
Answer: Allow nesting up to a small repeat budget
Confidence: 0.60