Skip to content

Commit e90e4d1

Browse files
committed
fix: address CI findings on the reference-integrity rules (#3583)
- Raw NUL byte in the hook-dedup key (validate-expressions.ts) tripped `check:nul-bytes`. A raw NUL makes grep/ripgrep treat the whole file as binary and silently return zero matches, so the file drops out of code search and every grep-based lint. Written as the \u0000 escape per the repo convention — byte-identical at runtime. - CodeQL (high): the `{…}` interpolation test used /\{[^}]+\}/, which backtracks quadratically on a long run of `{` with no closing brace. Replaced with a linear two-index scan; same semantics, including the "at least one character between the braces" rule. - spec public API gained 6 exports (the platform-object registry), so the api-surface snapshot needed regenerating. 0 breaking, 6 added. Also wire the two rules into `os compile` alongside `validate`/`lint`. Every sibling reference rule (widget bindings, dashboard action refs, filter tokens) already runs there, so leaving these out would recreate the exact validate/lint/compile wiring drift the assessment flagged. Document the new checks in docs/deployment/validating-metadata.mdx — a "Dangling object and action names" section with the resolvability severity table, plus the entry-points row. Suites: lint 390, spec 6669, cli 636 passing; eslint, nul-bytes and api-surface checks clean. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01GBks5G7AkwrvgF2kL5rfkT
1 parent 8ca8212 commit e90e4d1

5 files changed

Lines changed: 93 additions & 3 deletions

File tree

content/docs/deployment/validating-metadata.mdx

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -83,6 +83,39 @@ header: {
8383
whose `objects/reports/dashboards/pages/views` route is unregistered is **warned**
8484
(external, interpolated, and opaque routes are skipped).
8585

86+
### 4. Dangling object and action names
87+
88+
The same gate covers the reference sites that are plain strings in the schema:
89+
an action param's record-picker target, a dashboard filter's options source, a
90+
navigation capability gate, and every surface that binds an action **by name**
91+
(`bulkActions` / `rowActions`, a page's `record:quick_actions`, a nav action item).
92+
93+
```ts
94+
// ✗ the platform user object is `sys_user` — `user` resolves to nothing → error
95+
{ name: 'owner', type: 'lookup', reference: 'user' }
96+
97+
// ✗ no action named `mass_update` is defined anywhere → error
98+
defineView({ /**/ bulkActions: ['mass_update', 'mass_delete'] })
99+
100+
// ⚠ platform-shaped, but no known package registers it → warning
101+
{ id: 'nav_approvals', type: 'object', objectName: 'sys_approval_process',
102+
requiresObject: 'sys_approval_process' }
103+
```
104+
105+
Severity follows **resolvability**, because "this might be provided by another
106+
installed package" is a real possibility that must not be guessed at:
107+
108+
| The name… | Result |
109+
|---|---|
110+
| resolves to one of your own objects ||
111+
| is unresolved and carries no platform prefix | **error** — your objects are namespace-prefixed and present in the stack, so there is no legitimate elsewhere. This is the typo class (`user` for `sys_user`). |
112+
| resolves to a known platform / plugin / cloud object ||
113+
| carries a platform prefix but no known package registers it | **warning** — a third-party package may still provide it, so this stays advisory |
114+
115+
Interpolated targets (`${…}`, `{…}`) are skipped — they resolve at render time.
116+
Action names get no third-party softening: the runtime ships no built-in action
117+
names, so a name resolving nowhere is always an **error**.
118+
86119
## The one gate, two entry points
87120

88121
`os validate` and `os build` (alias of `os compile`) run the **same** validator:
@@ -93,6 +126,7 @@ whose `objects/reports/dashboards/pages/views` route is unregistered is **warned
93126
| CEL / predicate validation |||
94127
| Widget-binding integrity |||
95128
| Dashboard action/route references (ADR-0049) |||
129+
| Object & action name references (#3583) |||
96130
| Security posture (ADR-0090 — e.g. every custom object declares `sharingModel`) |||
97131
| Emits `dist/objectstack.json` |||
98132

packages/cli/src/commands/compile.ts

Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@ import { validateVisibilityPredicates } from '@objectstack/lint';
1313
import { validateWidgetBindings } from '@objectstack/lint';
1414
import { validateDashboardActionRefs } from '@objectstack/lint';
1515
import { validateFilterTokens } from '@objectstack/lint';
16+
import { validateObjectReferences, validateActionNameRefs } from '@objectstack/lint';
1617
import { validateResponsiveStyles } from '@objectstack/lint';
1718
import { validateSecurityPosture, validateOrgAxisRedLines, buildAccessMatrix, diffAccessMatrix } from '@objectstack/lint';
1819
import { validateReadonlyFlowWrites } from '@objectstack/lint';
@@ -319,6 +320,43 @@ export default class Compile extends Command {
319320
this.exit(1);
320321
}
321322

323+
// 3b-bis. Object & action name references (#3583) — the reference sites
324+
// `defineStack` does not cover: action-param `reference` /
325+
// `objectOverride`, dashboard filter `optionsFrom.object`, nav
326+
// `requiresObject` gates, and the name-bound action surfaces
327+
// (`bulkActions`/`rowActions`, page quick-actions, nav action items).
328+
// All plain strings in the schema, so a name resolving to nothing
329+
// ships and fails silently. Errors fail the build; the
330+
// platform-prefixed-but-unregistered case is advisory (a third-party
331+
// package may still provide it).
332+
if (!flags.json) printStep('Checking object & action references (#3583)...');
333+
const refFindings = [
334+
...validateObjectReferences(result.data as Record<string, unknown>),
335+
...validateActionNameRefs(result.data as Record<string, unknown>),
336+
];
337+
const refErrors = refFindings.filter((f) => f.severity === 'error');
338+
const refWarnings = refFindings.filter((f) => f.severity === 'warning');
339+
if (refErrors.length > 0) {
340+
if (flags.json) {
341+
console.log(JSON.stringify({ success: false, error: 'reference integrity validation failed', issues: refErrors }));
342+
this.exit(1);
343+
}
344+
console.log('');
345+
printError(`Reference integrity check failed (${refErrors.length} issue${refErrors.length > 1 ? 's' : ''})`);
346+
for (const f of refErrors.slice(0, 50)) {
347+
console.log(` • ${f.where}: ${f.message}`);
348+
console.log(chalk.dim(` ${f.hint}`));
349+
console.log(chalk.dim(` rule: ${f.rule} at ${f.path}`));
350+
}
351+
this.exit(1);
352+
}
353+
if (!flags.json) {
354+
for (const w of refWarnings.slice(0, 50)) {
355+
console.log(chalk.yellow(` ⚠ ${w.where}: ${w.message}`));
356+
console.log(chalk.dim(` ${w.hint}`));
357+
}
358+
}
359+
322360
// 3c. SDUI scoped-styling correctness (ADR-0065) — a styled node without
323361
// an `id` drops its CSS silently; Tailwind-in-className does nothing
324362
// from metadata. Same bar for hand-authored and AI-generated pages

packages/lint/src/validate-expressions.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -287,7 +287,7 @@ export function validateStackExpressions(stack: AnyRec): ExprIssue[] {
287287
check(`hook '${hookName}' (${target}) condition`, hook.condition, target, 'record');
288288
for (let i = mark; i < issues.length; i++) {
289289
const issue = issues[i];
290-
const key = `${issue.message}
290+
const key = `${issue.message}\u0000${issue.source ?? ''}`;
291291
// Keep the first occurrence of each distinct diagnostic. A field-unknown
292292
// finding differs per target (it names the object), so each survives;
293293
// a syntax error is identical across targets and collapses to one.

packages/lint/src/validate-object-references.ts

Lines changed: 14 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -102,9 +102,21 @@ function strName(v: unknown): string | undefined {
102102
return typeof v === 'string' && v.length > 0 ? v : undefined;
103103
}
104104

105-
/** A target the author cannot have meant literally — resolved at render time. */
105+
/**
106+
* A target the author cannot have meant literally — `${…}` or `{…}` resolved at
107+
* render time.
108+
*
109+
* Scanned rather than matched with `/\{[^}]+\}/`: that pattern backtracks
110+
* quadratically on a long run of `{` with no closing brace (CodeQL
111+
* polynomial-ReDoS), and the question here is simply "is there a `{` with a
112+
* later `}` and something in between", which one pass answers.
113+
*/
106114
function isInterpolated(target: string): boolean {
107-
return target.includes('${') || /\{[^}]+\}/.test(target);
115+
if (target.includes('${')) return true;
116+
const open = target.indexOf('{');
117+
// `+ 2` keeps the original "at least one character between the braces"
118+
// semantics, so a literal `{}` is not treated as a placeholder.
119+
return open !== -1 && target.indexOf('}', open + 2) !== -1;
108120
}
109121

110122
/** Levenshtein-bounded "did you mean?" over the known names. */

packages/spec/api-surface.json

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -649,6 +649,7 @@
649649
"BookSchema (const)",
650650
"BucketConfig (type)",
651651
"BucketConfigSchema (const)",
652+
"CLOUD_PROVIDED_OBJECT_NAMES (const)",
652653
"CRDTMergeResult (type)",
653654
"CRDTMergeResultSchema (const)",
654655
"CRDTState (type)",
@@ -1006,6 +1007,9 @@
10061007
"OpenTelemetryCompatibilitySchema (const)",
10071008
"OtelExporterType (type)",
10081009
"PKG_CONVENTIONS (const)",
1010+
"PLATFORM_OBJECTS_BY_PACKAGE (const)",
1011+
"PLATFORM_OBJECT_PREFIXES (const)",
1012+
"PLATFORM_PROVIDED_OBJECT_NAMES (const)",
10091013
"PNCounter (type)",
10101014
"PNCounterSchema (const)",
10111015
"PackageDirectory (type)",
@@ -1260,6 +1264,8 @@
12601264
"docAudienceAllows (function)",
12611265
"emailTemplateForm (const)",
12621266
"gcsStorageExample (const)",
1267+
"hasPlatformObjectPrefix (function)",
1268+
"isPlatformProvidedObjectName (function)",
12631269
"isPublicAudience (function)",
12641270
"minioStorageExample (const)",
12651271
"resolveActionConfirm (function)",

0 commit comments

Comments
 (0)