Skip to content

fix: accept {n,} patterns and localize setup-field hints - #606

Merged
Weegy merged 1 commit into
mainfrom
fix/om-17-pattern-followups
Aug 3, 2026
Merged

fix: accept {n,} patterns and localize setup-field hints#606
Weegy merged 1 commit into
mainfrom
fix/om-17-pattern-followups

Conversation

@Weegy

@Weegy Weegy commented Aug 3, 2026

Copy link
Copy Markdown
Contributor

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 +

screenPatternSource rejected 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_REPETITION now applies to max(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:

pattern subject time
^[a-z]{1,100000}[a-z]{1,100000}$ 8191 non-matching chars 71 ms
^[a-z]+[a-z]+$ (always been allowed) same 39 ms
^a{100000,}$ compile + match 100k chars 0.43 ms

A 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.

Known conservatism worth a follow-up: [0-9a-f]{128} — a 512-bit hex key, a plausible credential — is still refused at 100. Visibly, via pattern_unavailable, not silently.

The client had the same bug, and there it was worse

web-ui/app/_lib/setupFieldPattern.ts carries 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 native pattern= attribute, and violatesSetupPattern fails 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

checkSetupFieldPattern picked the hint via pickPatternHint(field.pattern_hint, locale), but runtime.ts called it with three positional arguments and never passed a locale — so every 400 runtime.setup_field_invalid carried English regardless of UI language. installService had the same gap.

Bounded impact — CredentialsEditor localizes 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's hint as fallback when the field is unknown. New resolveSetupFieldHint(); used by the post-install editor and the install wizard alike.

No API change. hint stays 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-Language read anywhere in middleware/src, and LOCALE_COOKIE/NEXT_LOCALE never 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" mistake setupFieldPattern.ts warns against, in a different costume. The unused locale parameter was removed rather than left implying a threading that does not exist.

One small type change followed: FieldRow's error prop went from string to { code?, message }, because for a pattern_mismatch the server's message is the English hint and the component cannot otherwise tell it apart. Only pattern_mismatch is overridden; required / wrong_type render verbatim. No new i18n keys — the text is manifest content.

Verification

Mutation-tested, all five red before green:

mutation result
restore the {n,} rejection (middleware) 8 fail — incl. {2,}, {100,}, the end-to-end match, the realistic-manifest test
drop the new min cap only 2 fail^a{101,}$, ^a{100000,}$
restore the {n,} rejection (client mirror) 6 fail
resolveSetupFieldHint → return the server hint 3 fail — German-preferred, English-operator, cross-locale fallback
FieldRow → always render error.message 2 fail — German error in wizard and in editor

Loaded the real Google Workspace manifest through loadManifestFromPath (read-only, throwaway script — no test depends on that path): all four fields pattern_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.

middleware   typecheck clean · 5495 pass / 0 fail / 4 skipped   (was 5471)
web-ui       typecheck clean · lint 0 errors (38 pre-existing warnings)
             i18n:check OK — 3339 keys, en + de (unchanged)
             vitest 68 files / 552 tests pass                    (was 526)

One unrelated thing found, not fixed here

Driving setupFieldPattern.ts standalone under node --import tsx --input-type=module -e, every matchWithBudget call 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 on main, and the node --test harness 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.


View with [code]smith Autofix with [code]smith
Need help on this PR? Tag @codesmith-bot with what you need. Autofix is disabled.

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
Weegy merged commit 644e7fa into main Aug 3, 2026
9 checks passed
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.
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