Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
22 changes: 22 additions & 0 deletions .changeset/18066-meta-item-absent-404.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
---
'@objectstack/rest': patch
---

`GET /api/v1/meta/:type/:name` answers `404 RESOURCE_NOT_FOUND` for a name with nothing behind it, instead of `200` carrying the declared envelope minus its `item` member (#18066).

Measured on a real server (`examples/app-showcase`, API 17.4.0, four absent names, all identical):

```
GET /api/v1/meta/app/no_such_app_xyz
200 {"type":"app","name":"no_such_app_xyz","lock":"none","editable":true,"deletable":true,"resettable":false}
```

Two declarations in this repository already said otherwise, and this restores what they declare rather than deciding anything new. `GetMetaItemResponseSchema` — the route's own `responseSchema` — makes `item` a required member; parsing the body above against it fails `invalid_type` / `expected: 'nonoptional'` at `item`. And the **cached** arm of this same route has always answered this condition with `404 RESOURCE_NOT_FOUND`, because `getMetaItemCached` throws on a falsy `item`. Which arm a request took was deciding whether absence was an error at all — `app`, `dashboard`, `doc`, `book`, `?state=draft`, `?preview=draft`, `?package=` and every `enableCache: false` deployment are diverted around the cache.

- **Every type is affected, not only `app`.** The fall-through sat in the shared tail of the uncached arm, below the per-type gates. The report measured `app` because that type bypasses the cache structurally; a `?state=draft` or `?package=` read of any type reached the same 200.
- ⚠️ **The break was at `JSON.stringify`, not in the producer.** `metadata-protocol`'s `getMetaItem` returns `{ type, name, item: undefined, lock, … }` for a miss — `item` is *present* holding `undefined`, which `z.unknown()` admits — so the returned object conforms and only the serialized body does not. A conformance probe written against the object rather than the wire bytes reports agreement.
- **The permission denial is unchanged.** `403 PERMISSION_DENIED` for an app that exists and whose `requiredPermissions` the session lacks answers exactly as before: the new check is ordered ahead of every gate, and those gates are reachable only by a document that exists, so an absent name can never be converted into a denial. Enumerating app names through the 403 stays impossible.
- **It also closes an enumeration hole in the other direction.** ADR-0045 §3 makes an unpublished app *externally unobservable*, and an unpublished app answered this 404 while a nonexistent name answered the 200 — so the pair of responses reported which app names exist-but-are-unpublished. Both absence answers now come from one emitter and are byte-identical.
- **An unreadable metadata store is still `503`, never this 404.** That distinction is a producer-side throw and never reaches the new check.

⚠️ **For callers**: a probe that read "the call did not throw" as "this name resolves" now sees the 404 it should always have seen. A caller that read the item-less 200 as a create-vs-edit signal must read the status instead. The console side was already corrected independently (objectui#9262 reads both dialects as absence), so no first-party consumer depends on the old shape.
60 changes: 53 additions & 7 deletions packages/rest/src/meta-app-publish-gate.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -112,9 +112,24 @@ function setup(perms: string[], apps: any[] = ALL_APPS, serviceExists?: (name: s
const t = String(type ?? '');
return t === 'app' || t === 'apps' ? JSON.parse(JSON.stringify(apps)) : [];
}),
// [#18066] The MISS answers what the live producer answers, not
// `undefined`. `metadata-protocol`'s `getMetaItem` resolves the
// protection envelope AROUND a `item: undefined` for a name it cannot
// find — `lock` / `editable` / `deletable` / `resettable` come from
// `resolveLockState` unconditionally — so `undefined` was a shape no
// deployment produces. That mattered: the door read `envelope?.item`,
// which is `undefined` for both, and then fell through to `res.json`
// with the envelope, so the criterion-3 case below was passing over a
// stub that could not exhibit the defect the live provider had. The
// reject-path case further down keeps its own override, so both
// producer shapes reach this route from this file.
getMetaItem: vi.fn(async ({ name }: any) => {
const found = apps.find((a: any) => a.name === name);
return found ? { type: 'app', name, item: JSON.parse(JSON.stringify(found)) } : undefined;
return {
type: 'app', name,
item: found ? JSON.parse(JSON.stringify(found)) : undefined,
lock: 'none', editable: true, deletable: true, resettable: false,
};
}),
findData: vi.fn().mockResolvedValue([]),
};
Expand Down Expand Up @@ -447,15 +462,46 @@ describe('#8013 — by-name: a permission denial is REPORTED, absence still is n
expect(missing.statusCode).not.toBe(403);
expect(refusal(missing.body).code).not.toBe('PERMISSION_DENIED');
expect(JSON.stringify(missing.body ?? {})).not.toContain('PERMISSION_DENIED');

// [#18066] The POSITIVE half, which this case did not state and
// which is what let the route answer `200` with an item-less
// envelope for years while every assertion above stayed green:
// "not the denial" was satisfied by a SUCCESS just as well as by an
// absence. ADR-0112 — `status` and `code`.
expect(missing.statusCode).toBe(404);
expect(refusal(missing.body).code).toBe('RESOURCE_NOT_FOUND');
expect(missing.body?.item).toBeUndefined();
expect(missing.body?.lock).toBeUndefined();
}
});

it('criterion 3: …and the real producer miss is still the 404 it has always been', async () => {
// The fixture's `getMetaItem` answers `undefined` for an unknown name;
// `metadata-protocol` REJECTS with a declared `RESOURCE_NOT_FOUND` /
// `status: 404` (pinned in `rest-meta-outage-vs-miss.test.ts`). Both
// reach this route, so the criterion is stated against the production
// shape too rather than against the stub's alone.
it('criterion 3: …and it is the SAME answer the unpublished app gets, byte for byte', async () => {
// [#18066] ADR-0045 §3 makes an unpublished app externally
// unobservable, and this suite's partition note states the contract as
// absence and nonexistence being indistinguishable. Over the live
// producer shape they were not: `production_management` answered this
// 404 while `no_such_app` answered a 200 envelope, so the pair
// enumerated which app names exist-but-are-unpublished. Compared as
// whole bodies rather than field by field, because a single extra key
// on either side is the entire signal.
const unpublished = await getItem(setup(['manage_users'], GATED_APPS).rest, 'production_management');
const absent = await getItem(setup(['manage_users'], GATED_APPS).rest, 'no_such_app');

expect(absent.statusCode).toBe(unpublished.statusCode);
expect(absent.body).toEqual(unpublished.body);
expect(absent.statusCode).toBe(404);
});

it('criterion 3: …and the REJECTING producer shape reaches the same status and code', async () => {
// The other producer shape this door must survive: a protocol
// implementation that REJECTS with a declared `RESOURCE_NOT_FOUND` /
// `status: 404` (`rest-meta-outage-vs-miss.test.ts` pins the rendering).
// ⚠️ Its body is the FLAT `{ error: '<message>', code }` that
// `resolveErrorResponse`'s declared-status passthrough produces, not the
// nested ADR-0112 envelope the in-route refusals emit — so this case
// asserts `body.code`, and the case above asserts `body.error.code`, on
// purpose. Both reach this route, so the criterion is stated against
// both rather than against one stub's.
const { rest, protocol } = setup([], GATED_APPS);
protocol.getMetaItem = vi.fn().mockRejectedValue(Object.assign(
new Error('Metadata item app/no_such_app not found'),
Expand Down
Loading
Loading