From 1c5b9027923beeafcb1e39388397d8790350e55d Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 17 Sep 2026 02:07:06 +0000 Subject: [PATCH 1/2] fix(spec): refuse `requiresFeature` composed with a blank-`source` CEL `visible` MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `lowerRequiresFeature`'s guard tested the TYPE of `existing.source`, so a whitespace-only source — legal on the persistence contract, whose `min(1)` whitespace clears — passed it and the feature gate was composed around a blank operand, yielding `( ) && features.X != false`. That predicate parses on no scope at all, so the gate faults at evaluation instead of gating: the flag decides nothing whichever way the consuming surface fails. That is the parses-clean-changes-nothing arrival the guard's own docblock cites ADR-0078 to reject, produced by the composition step itself. The lowering now refuses a `source` that is blank after trimming, on the same leg as the AST-only refusal, with a refusal naming the composition it would have produced and both exits. The notion of blank is `source.trim()`, the one the engine's own helpers apply, so a merely padded source still composes. Claude-Session: https://claude.ai/code/session_01JbZnqu8bt6YqfJsr9vaFb3 Co-authored-by: Claude --- .../17631-requires-feature-blank-source.md | 23 +++++++++++++ .../src/kernel/public-auth-features.test.ts | 32 +++++++++++++++++++ .../spec/src/kernel/public-auth-features.ts | 24 ++++++++++++++ 3 files changed, 79 insertions(+) create mode 100644 .changeset/17631-requires-feature-blank-source.md diff --git a/.changeset/17631-requires-feature-blank-source.md b/.changeset/17631-requires-feature-blank-source.md new file mode 100644 index 00000000000..45e90525f1e --- /dev/null +++ b/.changeset/17631-requires-feature-blank-source.md @@ -0,0 +1,23 @@ +--- +'@objectstack/spec': patch +--- + +fix(spec): `requiresFeature` refuses a blank-`source` CEL `visible` instead of composing a predicate that can never parse (#17631) + +Clause-②: no + +`lowerRequiresFeature` lowers the `requiresFeature: ''` sugar into the canonical `visible` CEL predicate, and its own docblock states the ADR-0078 rule it enforces: a composition that could never take effect is a loud parse error, not a silent one. The guard that enforced it tested the TYPE of `source` (`typeof existing.source !== 'string'`), so a whitespace-only `source` — legal on `ExpressionSchema`, which is the persistence contract and whose `min(1)` whitespace clears — passed it and the gate was composed AROUND a blank operand: + +``` +visible: { dialect: 'cel', source: ' ' } + requiresFeature: 'organization' + → { dialect: 'cel', source: '( ) && features.organization != false' } +``` + +That predicate parses on no scope at all (`celEngine.evaluate` answers `kind: parse`, `Unexpected token: RPAREN`), so at render the gate faults instead of gating: fail-soft surfaces show the element regardless of the flag, fail-closed surfaces hide it regardless of the flag. Either way the flag decides nothing — the parses-clean-changes-nothing arrival the guard exists to reject, produced by the guard's own composition step. + +The lowering now refuses a `source` that is blank after trimming, on the same leg as the AST-only refusal one line above, with a refusal that names the composition it would have produced and both exits (drop the blank `visible` and the sugar emits the gate alone; or write the predicate the gate should compose with). The notion of blank is `source.trim()` — the one the engine's own helpers apply — so a `source` that is merely padded around real text still composes verbatim. + +- **Refused at the producer, not tolerated at a consumer.** No renderer gains a fallback for the unparseable predicate; the lowering stops emitting it. +- **Both slots that compose the sugar inherit it** — `ActionSchema.visible` and `ActionParamSchema.visible` — because the rule lives in the shared lowering rather than in either slot's declaration. +- **`ExpressionSchema` / `ExpressionInputSchema` are NOT narrowed.** They remain the persistence contract, and a blank-`source` `visible` with no `requiresFeature` beside it still parses exactly as before. What is refused is the COMPOSITION, which is the thing that could never work. +- **Nothing that functioned stops functioning.** The only authoring this refuses is one whose output faulted at CEL parse on every scope, so the migration is the refusal's own prescription and there is no working shape to port. diff --git a/packages/spec/src/kernel/public-auth-features.test.ts b/packages/spec/src/kernel/public-auth-features.test.ts index bca78079d5a..69b9a595fc5 100644 --- a/packages/spec/src/kernel/public-auth-features.test.ts +++ b/packages/spec/src/kernel/public-auth-features.test.ts @@ -147,4 +147,36 @@ describe('lowerRequiresFeature', () => { expect(issues[0]).toMatchObject({ code: 'custom', path: ['requiresFeature'] }); } }); + + // The same ADR-0078 leg, one step further in: a blank `source` is a STRING, + // so it passes the type test above and used to compose into + // `( ) && ` — a predicate no CEL parse accepts on any scope, so the + // flag decided nothing at render. Same shape as the pin above: kind and + // subject, never the wording. + it('rejects a CEL visible whose source is blank after trimming (ADR-0078)', () => { + for (const visible of [ + { dialect: 'cel', source: ' ' }, + { dialect: 'cel', source: '' }, + { dialect: 'cel', source: '\n\t' }, + ]) { + const { ctx, issues } = noIssues(); + const out = lowerRequiresFeature({ requiresFeature: 'admin' as const, visible }, ctx); + expect(issues).toHaveLength(1); + expect(issues[0]).toMatchObject({ code: 'custom', path: ['requiresFeature'] }); + // Refused, not composed — no `( ) && …` envelope is produced. + expect(out.visible).toEqual(visible); + } + }); + + // The boundary the refusal must NOT cross: a source that is merely PADDED is + // authored, and still composes verbatim (the engine trims its own input). + it('still composes a source that has leading/trailing whitespace around real text', () => { + const { ctx, issues } = noIssues(); + const out = lowerRequiresFeature( + { requiresFeature: 'admin' as const, visible: { dialect: 'cel', source: ' a ' } }, + ctx, + ); + expect(issues).toHaveLength(0); + expect(out.visible).toEqual({ dialect: 'cel', source: '( a ) && features.admin == true' }); + }); }); diff --git a/packages/spec/src/kernel/public-auth-features.ts b/packages/spec/src/kernel/public-auth-features.ts index e5b184f0f06..59e27e146a0 100644 --- a/packages/spec/src/kernel/public-auth-features.ts +++ b/packages/spec/src/kernel/public-auth-features.ts @@ -333,6 +333,15 @@ type WithRequiresFeature = { * Drop one of the two rather than shipping a gate that reads as load-bearing. * - Existing `visible` that is non-CEL or AST-only → loud parse error * (ADR-0078 no-silently-inert); write the combined predicate by hand. + * - Existing CEL `visible` whose `source` is blank after trimming → loud parse + * error, for the same reason one step further in. `source` is `min(1)` on the + * persistence contract and whitespace clears it, so a blank one is a *string* + * and would compose: the result parenthesises nothing (`( ) && `) and + * faults at CEL parse on every scope, so the gate decides nothing wherever + * the consuming surface is fail-soft and hides the element regardless of the + * flag wherever it is fail-closed — the inert arrival again, reached through + * the one spelling that passes the type test above. The notion of blank is + * `source.trim()`, the one the engine's own helpers apply. * * Designed as a zod `.transform((v, ctx) => lowerRequiresFeature(v, ctx))` * appended after the schema's refinements. @@ -376,6 +385,21 @@ export function lowerRequiresFeature( }); return rest as Omit; } + if (existing.source.trim().length === 0) { + ctx.addIssue({ + code: 'custom', + path: ['requiresFeature'], + message: + '`requiresFeature` composes only with a CEL `visible` carrying a NON-BLANK `source`; this ' + + '`source` is blank after trimming, so the composition would parenthesise nothing — ' + + `\`( ) && ${gate}\` — which no CEL parse accepts on any scope. The gate would fault at ` + + 'evaluation instead of gating: shown regardless of the flag where the surface is fail-soft, ' + + 'hidden regardless of it where it is fail-closed. Either way the flag decides nothing, which ' + + 'is the inert arrival ADR-0078 rejects. Drop the blank `visible` and `requiresFeature` emits ' + + `\`${gate}\` alone, or put the predicate the gate should compose with in \`source\`.`, + }); + return rest as Omit; + } return { ...rest, visible: { ...existing, source: `(${existing.source}) && ${gate}` }, From ec0eaed2d2053cd63da2db9c5efd0a17ba9e5e9c Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 17 Sep 2026 02:29:32 +0000 Subject: [PATCH 2/2] fix(spec): keep the blank-`source` refusal out of `check:doc-authoring`'s sink closure MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Interpolating the composed gate into the refusal put `featureGatePredicate` — and through it the whole `PUBLIC_AUTH_FEATURES` registry — in a customer-facing `message:` position, and that gate's per-module fixed point then swept the registry's internal `notes` / `exempt.reason` prose as customer-facing text. Measured: `pnpm check:doc-authoring` is green at the base commit and flagged three pre-existing strings with the interpolation in. The sentence now names the composed shape without interpolating the gate, which is also how the two refusals beside it are written. No test pinned the wording. Claude-Session: https://claude.ai/code/session_01JbZnqu8bt6YqfJsr9vaFb3 Co-authored-by: Claude --- .../spec/src/kernel/public-auth-features.ts | 21 +++++++++++++------ 1 file changed, 15 insertions(+), 6 deletions(-) diff --git a/packages/spec/src/kernel/public-auth-features.ts b/packages/spec/src/kernel/public-auth-features.ts index 59e27e146a0..ca6f8dd0ffa 100644 --- a/packages/spec/src/kernel/public-auth-features.ts +++ b/packages/spec/src/kernel/public-auth-features.ts @@ -389,14 +389,23 @@ export function lowerRequiresFeature( ctx.addIssue({ code: 'custom', path: ['requiresFeature'], + // ⚠ Deliberately does NOT interpolate `gate`. Doing so puts + // `featureGatePredicate` — and through it the whole + // `PUBLIC_AUTH_FEATURES` registry — in a customer-facing message + // position, and `check:doc-authoring`'s per-module fixed point then + // sweeps that registry's INTERNAL `notes` / `exempt.reason` prose as + // customer-facing text (measured: green at the base commit, three + // pre-existing strings flagged with the interpolation in). The concrete + // gate is one `featureGatePredicate` call away for anyone who wants it. message: '`requiresFeature` composes only with a CEL `visible` carrying a NON-BLANK `source`; this ' - + '`source` is blank after trimming, so the composition would parenthesise nothing — ' - + `\`( ) && ${gate}\` — which no CEL parse accepts on any scope. The gate would fault at ` - + 'evaluation instead of gating: shown regardless of the flag where the surface is fail-soft, ' - + 'hidden regardless of it where it is fail-closed. Either way the flag decides nothing, which ' - + 'is the inert arrival ADR-0078 rejects. Drop the blank `visible` and `requiresFeature` emits ' - + `\`${gate}\` alone, or put the predicate the gate should compose with in \`source\`.`, + + '`source` is blank after trimming, so composing the feature gate onto it would parenthesise ' + + 'nothing — the predicate would read `( ) && ` followed by the gate — which no CEL parse ' + + 'accepts on any scope. The gate would fault at evaluation instead of gating: the element is ' + + 'shown regardless of the flag where the consuming surface is fail-soft and hidden regardless ' + + 'of it where it is fail-closed, so the flag decides nothing — the inert arrival ADR-0078 ' + + 'rejects. Drop the blank `visible` and `requiresFeature` emits the gate alone, or put the ' + + 'predicate the gate should compose with in `source`.', }); return rest as Omit; }