Skip to content

Commit 6b7129a

Browse files
os-zhuangclaude
andauthored
fix(rest,lint,spec): prune nav entries whose destination object cannot serve, and refuse them at authoring time (#7912) (#8426)
* fix(rest,lint,spec): prune nav entries whose destination object cannot serve, and refuse them at authoring time (#7912) A `type: 'object'` nav entry pointing at an object that cannot answer a list was served in the `/meta` payload anyway. Two independent conditions make a destination unservable — `enable.apiEnabled: false` (404) and an `enable.apiMethods` whitelist without `list` (405) — and both are pure functions of the object's own `enable` block, so the destination is dead for every persona. That is why no `requiredPermissions` gate on the entry could ever prune it. Per the maintainer ruling of 2026-08-12 (option (c)): derive the fact, mint no new key. - `filterAppForUser` consults the destination's `enable` for every `type: 'object'` entry, on both `/meta` app routes and inside `children` and `areas[]`. Fail-open on unknown objects and unreadable metadata, so `requiresObject`'s client-only pin and #3770 both stand. - The mandatory companion: `os validate` / `os build` / `os lint` now fail with `nav-object-unservable`, naming the entry, the object, the offending `enable` key path and which condition fired. The serving side logs the same facts. - The two-step gate order is declared once as `apiExposureDenialReason` / `canServeApiOperation` in `@objectstack/spec/data`. The REST data gate, the nav prune and #7909's invariant test all read that one export instead of re-spelling it — the third spelling is what this extraction removes. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01P7vaLs7bhBPi9m3JyzkhDj * chore(spec): regenerate api-surface / export-origins after merging origin/main (#7912) Both artifacts were merged without a text merge: this branch and #8342 each added exports to `@objectstack/spec/data`, so neither side's baseline covered the union. Regenerated from the merged tree, after rebuilding dist — the gate reads the BUILT dist, and on a stale one it reports phantom removals that the generator would then write. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01P7vaLs7bhBPi9m3JyzkhDj * fix(rest): give the #7912 nav-servability test's import an explicit .js extension (#7912) `check:type-check-debt` went red: @objectstack/rest's TEST_DEBT records 155 raw tsc errors and the re-measure reported 156 (+1). Attributed, not assumed. The package-level `typecheck` is genuinely clean — `packages/rest` is one of the 20 packages whose tsconfig hides its own tests from tsc, so nothing local can see the test layer. Replaying the ledger's own program (the package tsconfig with the test globs dropped from `exclude`) gives 156, of which exactly ONE is in the new file and exactly 155 are not — matching the recorded number, so nothing pre-existing on main drifted: meta-app-nav-servability-gate.test.ts(22,28): error TS2835: Relative import paths need explicit file extensions in ECMAScript imports when '--moduleResolution' is 'node16' or 'nodenext'. Did you mean './rest-server.js'? The package resolves NodeNext, so the extensionless spelling the older test files use is a TS2835 — 67 of them are frozen in the ledger. The fix is the extension, so a new file does not add the 68th. ⛔ The TEST_DEBT entry was NOT raised, and `scripts/check-type-check-coverage.mjs` was not touched. The ledger is a ratchet: `--re-measure` now reports "none above its recorded number / surplus: none — every entry sits exactly at its measurement". Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01P7vaLs7bhBPi9m3JyzkhDj --------- Co-authored-by: Claude <noreply@anthropic.com>
1 parent 3f4b696 commit 6b7129a

11 files changed

Lines changed: 909 additions & 20 deletions

File tree

Lines changed: 60 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,60 @@
1+
---
2+
"@objectstack/spec": patch
3+
"@objectstack/rest": patch
4+
"@objectstack/lint": patch
5+
---
6+
7+
fix(rest,lint,spec): prune nav entries whose destination object cannot serve, and refuse them at authoring time (#7912)
8+
9+
A `type: 'object'` navigation entry pointing at an object that **cannot answer a
10+
list** was served to the client in the `/meta` payload anyway. The user saw a
11+
menu item that could not work, and the console rendered the failure as a generic
12+
empty state — so it read as *"you have no records"* rather than *"this page
13+
cannot work"*.
14+
15+
Two independent conditions make a destination unservable, and **neither was
16+
expressible on a nav entry**:
17+
18+
- `enable.apiEnabled: false` → the list answers `OBJECT_API_DISABLED` (404);
19+
- an `enable.apiMethods` whitelist without `list` → `OBJECT_API_METHOD_NOT_ALLOWED` (405).
20+
21+
Both are pure functions of the object's own `enable` block — no user, no
22+
permissions, no request context — so the destination is dead for **every**
23+
persona, platform administrator included. That is why a `requiredPermissions`
24+
gate could never prune such an entry: the two are independent conditions, and no
25+
combination of permissions on the *entry* rescues an entry whose *object* is
26+
API-disabled. One shipped that way for a year and read as correct to reviewers,
27+
its in-code comment claiming a non-admin "403s server-side" — which implies an
28+
admin could list. None could.
29+
30+
**The fact is now derived, not declared.** `filterAppForUser` consults the
31+
destination's `enable` block for every `type: 'object'` entry and drops the ones
32+
that cannot serve, on both the app-list and the by-name `/meta` routes and
33+
inside `children` and `areas[]` alike. No new authorable key was minted: the
34+
platform already knows this, on the object, in one place.
35+
36+
**And the prune is never silent.** A prune the author cannot see is the same
37+
failure one layer over, so it is refused at authoring time: `os validate` /
38+
`os build` / `os lint` now **fail** with `nav-object-unservable`, naming the
39+
entry, the object, the offending `enable` key path and which of the two
40+
conditions fired. The serving side logs the same facts for an entry that reaches
41+
a running deployment anyway.
42+
43+
The single two-step order these consumers share — `apiEnabled` first and
44+
independently, the whitelist second — is now declared once as
45+
`apiExposureDenialReason` / `canServeApiOperation` in `@objectstack/spec/data`,
46+
beside the `resolveEffectiveApiMethods` / `isApiOperationAllowed` primitives it
47+
composes. The REST data gate, the nav prune and the authoring rule all read that
48+
one export instead of re-spelling the order.
49+
50+
**Deliberately unchanged:**
51+
52+
- `requiresObject` keeps its client-only evaluation. It asks whether an object
53+
is *registered*; this gate asks whether a registered object's `enable` block
54+
lets it answer. An entry whose object this layer cannot find is **served**,
55+
not pruned.
56+
- `visible` (CEL) is still client-side only.
57+
- Fail-open throughout: unreadable object metadata prunes nothing, so a cold
58+
start or a metadata outage cannot empty a healthy deployment's sidebar.
59+
- Objects an authoring stack does not itself declare are not judged by the lint
60+
rule — their `enable` block is not visible from there.

‎packages/lint/src/index.ts‎

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -334,6 +334,16 @@ export type { ObjectRefFinding, ObjectRefSeverity } from './validate-object-refe
334334
export { validateNavTargetRefs, NAV_TARGET_UNRESOLVED } from './validate-nav-target-refs.js';
335335
export type { NavTargetRefFinding, NavTargetRefSeverity } from './validate-nav-target-refs.js';
336336

337+
// [#7912] The servability question about an `object` nav target: not "does the
338+
// name resolve?" but "can the destination answer a list at all?". Gates, and
339+
// gates alone among the nav rules — `enable` is declared on the object in this
340+
// same stack, so a finding is a certainty rather than a suspicion.
341+
export {
342+
validateNavObjectServability,
343+
NAV_OBJECT_UNSERVABLE,
344+
} from './validate-nav-object-servability.js';
345+
export type { NavObjectServabilityFinding } from './validate-nav-object-servability.js';
346+
337347
export {
338348
validateSearchableFields,
339349
SEARCHABLE_FIELD_UNKNOWN,

‎packages/lint/src/reference-integrity-suite.test.ts‎

Lines changed: 21 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@ describe('reference-integrity suite — membership', () => {
2222
'validateChartBindings',
2323
'validateNavAccess',
2424
'validateNavTargetRefs',
25+
'validateNavObjectServability',
2526
'validateTranslationReferences',
2627
'validateTranslatableSections',
2728
'validateFlowTemplatePaths',
@@ -65,6 +66,18 @@ describe('reference-integrity suite — every member actually runs', () => {
6566
searchableFields: ['name', 'budget'],
6667
permissions: {},
6768
},
69+
// validateNavObjectServability (#7912): an object the app puts in its
70+
// navigation while its own `enable` block refuses every API operation.
71+
// A SEPARATE object from `crm_lead` on purpose — putting the dead
72+
// `enable` on the object every other member reads would let this one go
73+
// silent behind their findings, and would change what `nav_leads` means
74+
// to `validateNavAccess`.
75+
{
76+
name: 'crm_secret_token',
77+
fields: { name: { type: 'text', label: 'Name' } },
78+
enable: { apiEnabled: false, apiMethods: [] },
79+
permissions: {},
80+
},
6881
],
6982
actions: [
7083
// validateObjectReferences: a param pointing at an object nothing declares.
@@ -151,7 +164,13 @@ describe('reference-integrity suite — every member actually runs', () => {
151164
apps: [
152165
{
153166
name: 'crm_app',
154-
navigation: [{ id: 'nav_leads', type: 'object', objectName: 'crm_lead' }],
167+
navigation: [
168+
{ id: 'nav_leads', type: 'object', objectName: 'crm_lead' },
169+
// validateNavObjectServability: the destination answers 404
170+
// `OBJECT_API_DISABLED` for every persona, so the row is dead however
171+
// it is permissioned.
172+
{ id: 'nav_tokens', type: 'object', objectName: 'crm_secret_token' },
173+
],
155174
},
156175
],
157176
// validateNavAccess: a declared permission set that grants nothing on the
@@ -237,6 +256,7 @@ describe('reference-integrity suite — every member actually runs', () => {
237256
expect(rules).toContain('page-field-unknown');
238257
expect(rules).toContain('chart-measure-unknown');
239258
expect(rules).toContain('nav-object-ungranted');
259+
expect(rules).toContain('nav-object-unservable');
240260
expect(rules).toContain('translation-target-unknown');
241261
expect(rules).toContain('translation-section-name-missing');
242262
expect(rules).toContain('flow-template-unknown-field');

‎packages/lint/src/reference-integrity-suite.ts‎

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -61,6 +61,7 @@ import { validatePageFieldBindings } from './validate-page-field-bindings.js';
6161
import { validateChartBindings } from './validate-chart-bindings.js';
6262
import { validateNavAccess } from './validate-nav-access.js';
6363
import { validateNavTargetRefs } from './validate-nav-target-refs.js';
64+
import { validateNavObjectServability } from './validate-nav-object-servability.js';
6465
import { validateTranslationReferences } from './validate-translation-references.js';
6566
import { validateTranslatableSections } from './validate-translatable-sections.js';
6667
import { validateFlowTemplatePaths } from './validate-flow-template-paths.js';
@@ -120,6 +121,15 @@ export const REFERENCE_INTEGRITY_RULES: readonly ReferenceIntegrityRule[] = [
120121
// `action` is deliberately absent (validateActionNameRefs owns it) and so is
121122
// `component` (an unregistered ref renders a named diagnostic, not silence).
122123
{ name: 'validateNavTargetRefs', run: validateNavTargetRefs },
124+
// [#7912] The THIRD question about a nav entry, after "does the target
125+
// resolve?" (above) and "is it granted?" (`validateNavAccess`): can the
126+
// destination serve at all? An object's own `enable` block can make its list
127+
// answer 404/405 for every persona, and no gate authorable on the entry
128+
// expresses that — which is how #7544's dead row survived review for a year.
129+
// The server now prunes such an entry from the `/meta` payload; the
130+
// maintainer ruling of 2026-08-12 makes THIS the mandatory companion, so the
131+
// prune is never silent to the author who wrote the row.
132+
{ name: 'validateNavObjectServability', run: validateNavObjectServability },
123133
{ name: 'validateTranslationReferences', run: validateTranslationReferences },
124134
// The same family from the other end (#5417). Its sibling above asks "does
125135
// this bundle key resolve?"; this one asks "is there a key at all?" — a form
Lines changed: 202 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,202 @@
1+
// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license.
2+
3+
/**
4+
* [#7912] A navigation entry whose destination object cannot serve a `list` —
5+
* refused at authoring time, before it can be published and pruned in silence.
6+
*
7+
* ## The defect
8+
*
9+
* A `type: 'object'` nav entry names its destination in `objectName`. That
10+
* object's own `enable` block decides whether the external REST surface can
11+
* answer a `list` there at all:
12+
*
13+
* - `enable.apiEnabled: false` → `OBJECT_API_DISABLED` (404);
14+
* - an `enable.apiMethods` whitelist without `list` → `OBJECT_API_METHOD_NOT_ALLOWED` (405).
15+
*
16+
* Both are pure functions of `enable` — no user, no permissions, no request
17+
* context — so the destination is dead for EVERY persona, platform admin
18+
* included. #7544 shipped exactly such an entry for a year: it read as correct
19+
* to reviewers because the entry carried a `requiredPermissions` gate, and the
20+
* in-code comment claimed a non-admin "403s server-side", which implies an
21+
* admin could list. None could. The 404 precedes and ignores permissions
22+
* entirely, and no combination of permissions on the ENTRY can prune an entry
23+
* whose OBJECT is API-disabled — they are independent conditions.
24+
*
25+
* ## Why this rule exists even though the server now prunes
26+
*
27+
* The maintainer ruling of 2026-08-12 chose to DERIVE servability rather than
28+
* mint a new nav key: `filterAppForUser` (`@objectstack/rest`) drops these
29+
* entries from the `/meta` payload, so the user never sees a menu item that
30+
* cannot work. That fixes the user-facing half and opens an authoring-facing
31+
* one — the ruling names it and makes this rule a mandatory companion, not an
32+
* optional extra:
33+
*
34+
* > A prune the author cannot see is the same failure one layer over — no
35+
* > silent dead rows, and no silent repairs.
36+
*
37+
* An author whose object is accidentally API-disabled would otherwise watch the
38+
* entry vanish from a running app with no signal anywhere. This rule is the
39+
* signal, raised at the checkpoint that can still see the whole picture and
40+
* naming both halves: which entry, and the exact `enable` key that killed it.
41+
*
42+
* ## Severity: `error`, unlike its neighbours — and why that is not over-reach
43+
*
44+
* `validate-nav-access` (its closest sibling: "navigation exposes an object no
45+
* permission set grants") is advisory, because a grant can legitimately arrive
46+
* from a package this stack cannot see. Nothing analogous applies here.
47+
* `enable` is declared ON the object, in this stack, and this rule judges ONLY
48+
* objects this stack declares — so when it fires, it has read the whole of the
49+
* evidence and the entry is dead with certainty. There is no installed package
50+
* that can make an `apiEnabled: false` object listable.
51+
*
52+
* The exemption is therefore the same shape as the sibling's, drawn one axis
53+
* over: a target this stack does not declare is SKIPPED entirely rather than
54+
* guessed at, because its `enable` block is not visible from here.
55+
*
56+
* ## ⛔ What this rule deliberately does NOT judge
57+
*
58+
* - **Whether the object exists at all.** An unresolvable nav target is
59+
* `validate-object-references` / `defineStack`'s question, and on the
60+
* serving side it is `requiresObject`'s — a key whose client-only evaluation
61+
* the same ruling explicitly declined to re-mean. Silence here on an unknown
62+
* name is that boundary, not an oversight.
63+
* - **Permissions.** Ungranted-but-listable is `validate-nav-access`; the two
64+
* conditions are independent and each needs its own finding, which is the
65+
* load-bearing lesson of #7544.
66+
* - **Non-`object` entries.** A `component` / `page` / `url` entry has no
67+
* `objectName` destination to judge, even when it carries `requiresObject`.
68+
*/
69+
70+
import { canServeApiOperation, type EnableLike } from '@objectstack/spec/data';
71+
72+
import type { ReferenceIntegrityFinding } from './reference-integrity-suite.js';
73+
74+
export type NavObjectServabilityFinding = ReferenceIntegrityFinding;
75+
76+
/** Emitted when a nav entry targets an object whose `enable` block cannot serve a list. */
77+
export const NAV_OBJECT_UNSERVABLE = 'nav-object-unservable';
78+
79+
type AnyRec = Record<string, unknown>;
80+
81+
const isRec = (v: unknown): v is AnyRec => !!v && typeof v === 'object' && !Array.isArray(v);
82+
83+
/** Both authoring carriers: an array of documents, or a name-keyed map. */
84+
function asArray(v: unknown): AnyRec[] {
85+
if (Array.isArray(v)) return v.filter(isRec);
86+
if (isRec(v)) return Object.entries(v).map(([name, def]) => (isRec(def) ? { name, ...def } : { name }));
87+
return [];
88+
}
89+
90+
function strName(v: unknown): string | undefined {
91+
return typeof v === 'string' && v.length > 0 ? v : undefined;
92+
}
93+
94+
/**
95+
* An interpolated target resolves at render time — the same conservative
96+
* exemption `validate-object-references` and `validate-nav-target-refs` use to
97+
* keep false positives near zero (ADR-0072 D1).
98+
*/
99+
const isInterpolated = (s: string): boolean => s.includes('${') || s.includes('{');
100+
101+
export function validateNavObjectServability(stack: unknown): NavObjectServabilityFinding[] {
102+
const findings: NavObjectServabilityFinding[] = [];
103+
if (!isRec(stack)) return findings;
104+
105+
const apps = asArray(stack.apps);
106+
if (apps.length === 0) return findings;
107+
108+
// Only objects THIS stack declares can be judged — see the header. The map
109+
// records where each one is declared so a finding can point at the `enable`
110+
// key that is actually editable, not merely at the nav entry that tripped on
111+
// it.
112+
const ownEnable = new Map<string, { enable: EnableLike | undefined; path: string }>();
113+
const objects = asArray(stack.objects);
114+
for (const [oi, obj] of objects.entries()) {
115+
const n = strName(obj.name);
116+
if (!n) continue;
117+
// These rules read UNTYPED authored documents, so the shape is asserted
118+
// rather than proved. `EnableLike` is deliberately loose (every key
119+
// optional, plus an index signature) and `canServeApiOperation` treats a
120+
// missing/garbage block as "declares nothing" — so a non-object `enable`
121+
// reaches the default-open answer instead of throwing.
122+
ownEnable.set(n, { enable: obj.enable as EnableLike | undefined, path: `objects[${oi}].enable` });
123+
}
124+
if (ownEnable.size === 0) return findings;
125+
126+
for (const [ai, app] of apps.entries()) {
127+
const appName = strName(app.name) ?? `#${ai}`;
128+
129+
const walk = (items: unknown, basePath: string): void => {
130+
if (!Array.isArray(items)) return;
131+
for (const [ni, raw] of items.entries()) {
132+
if (!isRec(raw)) continue;
133+
const nav = raw;
134+
const navPath = `${basePath}[${ni}]`;
135+
136+
if (nav.type === 'object') {
137+
const target = strName(nav.objectName);
138+
const declared = target && !isInterpolated(target) ? ownEnable.get(target) : undefined;
139+
if (target && declared && !canServeApiOperation(declared.enable, 'list')) {
140+
const enable = isRec(declared.enable) ? declared.enable : {};
141+
// Which of the two conditions fired. `apiEnabled` is judged first
142+
// and independently — an API-disabled object refuses `list`
143+
// whatever its whitelist says — so the report follows the same
144+
// order rather than describing a whitelist the 404 never reaches.
145+
const apiDisabled = enable.apiEnabled === false;
146+
const condition = apiDisabled
147+
? '`enable.apiEnabled: false`'
148+
: '`enable.apiMethods` does not grant `list`'
149+
+ (Array.isArray(enable.apiMethods)
150+
? ` (declared: ${enable.apiMethods.length === 0 ? '[] — deny-all' : enable.apiMethods.map((m) => `\`${String(m)}\``).join(', ')})`
151+
: '');
152+
const answer = apiDisabled
153+
? '404 `OBJECT_API_DISABLED`'
154+
: '405 `OBJECT_API_METHOD_NOT_ALLOWED`';
155+
const offendingKey = apiDisabled
156+
? `${declared.path}.apiEnabled`
157+
: `${declared.path}.apiMethods`;
158+
159+
findings.push({
160+
severity: 'error',
161+
rule: NAV_OBJECT_UNSERVABLE,
162+
where: `app "${appName}" · nav "${strName(nav.id) ?? strName(nav.label) ?? `#${ni}`}"`,
163+
// The nav entry is where the dead row is authored; the `enable`
164+
// key that condemns it is named in the message, because the fix
165+
// may belong at either end.
166+
path: `${navPath}.objectName`,
167+
message:
168+
`Navigation targets object "${target}", which cannot serve a list: ${condition} `
169+
+ `(\`${offendingKey}\`), so the list request answers ${answer} for EVERY user — `
170+
+ `platform administrators included, since that gate reads only the object's \`enable\` `
171+
+ `block and never the caller. The entry cannot be rescued with `
172+
+ `\`requiredPermissions\`: they are independent conditions. The server prunes this `
173+
+ `entry from the served \`/meta\` payload (#7912), so publishing it ships a menu row `
174+
+ `that silently is not there.`,
175+
hint:
176+
`Remove the nav entry, or make "${target}" listable by setting \`enable.apiEnabled: true\` `
177+
+ `and granting \`list\` in \`enable.apiMethods\`. ⛔ Do NOT open the API on an object that `
178+
+ `is disabled on purpose — several platform objects hold credential material and are `
179+
+ `API-disabled deliberately; for those the entry is the mistake, not the \`enable\` block.`,
180+
});
181+
}
182+
}
183+
184+
// Recurse: an `object` nav item carries `children` too, not just a
185+
// `group` — the same reason `stack.zod.ts` does not gate its recursion
186+
// on the item type.
187+
if (Array.isArray(nav.children)) walk(nav.children, `${navPath}.children`);
188+
}
189+
};
190+
191+
walk(app.navigation, `apps[${ai}].navigation`);
192+
// `areas[]` is the other nav container, and the server gates it through the
193+
// very same walk (#4722) — so this rule must see it too, or it would pass a
194+
// stack whose served payload the runtime prunes.
195+
for (const [ari, area] of asArray(app.areas).entries()) {
196+
walk(area.items, `apps[${ai}].areas[${ari}].items`);
197+
walk(area.navigation, `apps[${ai}].areas[${ari}].navigation`);
198+
}
199+
}
200+
201+
return findings;
202+
}

0 commit comments

Comments
 (0)