fix: accept {n,} patterns and localize setup-field hints - #606
Merged
Conversation
Two defects in the setup-field validation shipped in #599, both found by writing the first real plugin manifest against it. Neither was visible from inside this repo. 1. The pattern allowlist refused `{n,}` while allowing `+`, which is the same thing. The first realistic pattern anyone wrote against the feature — `[A-Za-z]{2,}` for an email TLD — was rejected for no safety reason and had to be respelled. Shape is now judged purely by the group-content rules that already govern `+` and `*`, so every spelling of a quantifier is treated alike. Each hostile pattern in the existing table is now also asserted in its `{n,}` spelling, so the counted form cannot open a door the `+` form keeps shut. The size cap had to be extended to `{n,}`'s minimum, which nothing needed to bound while the construct was illegal — otherwise `a{100000,}` would have become legal the moment the shape check stopped rejecting it. The client carried a hand-mirrored copy of the same rule, and there the bug was worse: a rejected pattern means no native `pattern=` attribute and a check that fails open, so the operator would have typed a bad value, seen nothing, and hit a 400 from a validation the client had silently opted out of. Both halves are fixed together. 2. Every `400 runtime.setup_field_invalid` returned the English hint whatever the UI language, because the locale parameter was never passed. English help text in a German UI is one of the named contributing factors of the finding this feature exists to prevent. The client now resolves the hint from the field it already holds, keyed on the violation's field, and falls back to the server string when the field is unknown. No API change: `hint` stays the English fallback for clients that have no manifest. The unused locale parameter is gone rather than left implying a threading that does not exist. Middleware has no locale plumbing at all — no Accept-Language read anywhere, and the locale cookie never leaves the Next.js layer. Threading one would mean the server picking a language for a client it cannot see, which is the same untranslatable-string-through-the-API mistake in a different costume.
Weegy
added a commit
to byte5ai/omadia-google-workspace
that referenced
this pull request
Aug 3, 2026
The two subject-email patterns were written as `[A-Za-z][A-Za-z]+` only
because the platform's pattern allowlist refused `{n,}` while allowing the
equivalent `+`. byte5ai/omadia#606 fixed that inconsistency, so the bound
can be stated directly — and 63 is the real DNS label limit rather than an
open end.
Verified against the merged platform build: all four fields load with
pattern_unavailable=false, the refused-pattern registry stays empty for
them, and the value checks still behave — a real Workspace address and a
sub-domain address are accepted, `assistant@localhost` and `a@b.c` are
rejected, the service-account address still rejects a personal one, and the
private key still rejects a plain password.
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.
Two defects in the setup-field validation shipped in #599. Both were found by writing the first real plugin manifest against it (byte5ai/omadia-google-workspace#1), and neither was visible from inside this repo. Closes items 6 and 7 of #605.
1 · The allowlist refused
{n,}while allowing+screenPatternSourcerejected open-ended counted repetition with "open-ended counted repetition{n,}is not allowed" — while+, which is exactly{1,}, passed. Equivalent constructs, opposite verdicts: no safety bought, only author cost.Not hypothetical. The first realistic pattern written against this feature,
^[^@\s]+@[^@\s]+\.[A-Za-z]{2,}$for an email TLD, was refused and had to be respelled as[A-Za-z][A-Za-z]+. The agent who wrote the allowlist flagged the inconsistency at the time and complied with the spec anyway.Shape is now judged purely by the group-content rules that already govern
+and*, which fire on every quantifier spelling. Size checking stays separate.A guard had to be added, not just removed.
{n,}'s only number is the minimum, which nothing needed to bound while the construct was illegal.MAX_COUNTED_REPETITIONnow applies tomax(min, max)— without it,a{100000,}would have become legal the instant the shape check stopped rejecting it.Every hostile pattern is still rejected, and now also in its counted spelling —
^(a|a){1,}$,^(a{1,})+$,^((a{2,})){2,}$,^(?:a|a){2,}$,^(?=a{1,})b$— so the{n,}form cannot open a door the+form keeps shut.Measured, rather than assumed, on node 22:
^[a-z]{1,100000}[a-z]{1,100000}$^[a-z]+[a-z]+$(always been allowed)^a{100000,}$compile + matchA huge
{n,m}is the same polynomial class as the+this allowlist has always accepted — ~2×, not a new cliff — and V8 uses a counter rather than unrolling, so there is no compile-time blowup either. The cap is defence-in-depth; the 50 ms worker budget is the actual floor. The threshold stays at 100 because it covers every shape this feature exists for (DNS label ≤63, TLD 2–63, SHA-256 hex 64); raising it deserves its own evidence.The client had the same bug, and there it was worse
web-ui/app/_lib/setupFieldPattern.tscarries a hand-mirrored copy of the grammar including the identical{n,}rejection. Fixing only the server would have made a{2,}pattern unusable client-side: no nativepattern=attribute, andviolatesSetupPatternfails open. The operator types a bad value, sees nothing, hits Save, and eats a 400 from a check the client had silently opted out of. Both halves are fixed together.2 · The server always returned the English hint
checkSetupFieldPatternpicked the hint viapickPatternHint(field.pattern_hint, locale), butruntime.tscalled it with three positional arguments and never passed a locale — so every400 runtime.setup_field_invalidcarried English regardless of UI language.installServicehad the same gap.Bounded impact —
CredentialsEditorlocalizes correctly and blocks save before submitting, so the 400 is the fallback path (install wizard, API clients, any route bypassing the client check). But "English field labels and help texts in a German UI" is one of the named contributing factors of OM-17 itself, so shipping the fix with an English-only server message is a small own goal.Fix: the client resolves the hint from the field it already holds, keyed on the violation's
field, with the server'shintas fallback when the field is unknown. NewresolveSetupFieldHint(); used by the post-install editor and the install wizard alike.No API change.
hintstays the documented English fallback for clients that have no manifest (curl, install CLI, integrations), so an older client renders exactly what it renders today.Why not thread the locale server-side
Investigated and rejected: middleware has zero locale plumbing — no
Accept-Languageread anywhere inmiddleware/src, andLOCALE_COOKIE/NEXT_LOCALEnever leaves the Next.js layer. That option means building locale plumbing so the server can pick a language for a client it cannot see — the same "untranslatable string smuggled in through the API" mistakesetupFieldPattern.tswarns against, in a different costume. The unusedlocaleparameter was removed rather than left implying a threading that does not exist.One small type change followed:
FieldRow'serrorprop went fromstringto{ code?, message }, because for apattern_mismatchthe server'smessageis the English hint and the component cannot otherwise tell it apart. Onlypattern_mismatchis overridden;required/wrong_typerender verbatim. No new i18n keys — the text is manifest content.Verification
Mutation-tested, all five red before green:
{n,}rejection (middleware){2,},{100,}, the end-to-end match, the realistic-manifest testmincap only^a{101,}$,^a{100000,}${n,}rejection (client mirror)resolveSetupFieldHint→ return the server hintFieldRow→ always rendererror.messageLoaded the real Google Workspace manifest through
loadManifestFromPath(read-only, throwaway script — no test depends on that path): all four fieldspattern_unavailable=false, refused-pattern registry empty. Re-ran it with the workaround rewritten back to the{2,}the author actually wanted: still all four intact, registry still empty — so that plugin PR can drop its workaround once this lands.One unrelated thing found, not fixed here
Driving
setupFieldPattern.tsstandalone undernode --import tsx --input-type=module -e, everymatchWithBudgetcall overruns the 50 ms budget — even/^a$/against'a', at 84–212 ms. Once one overruns the worker is terminated, so the next call pays boot cost again and overruns too: a self-sustaining loop that fails every value closed. Reproduces identically with this branch's changes stashed, so it is onmain, and thenode --testharness does not hit it. Likely worker boot under tsx's loader exceeding the budget. Out of scope, but it would bite anything driving this module outside the test runner.Need help on this PR? Tag
@codesmith-botwith what you need. Autofix is disabled.