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
40 changes: 40 additions & 0 deletions .changeset/hono-retire-standard-endpoints.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,40 @@
---
"@objectstack/plugin-hono-server": major
---

feat(plugin-hono-server)!: delete the CRUD/discovery convenience surface and the `registerStandardEndpoints` flag — the plugin is a transport adapter (#4073)

Completes the retirement. `HonoServerPlugin` now owns the socket, the middleware
and the three current-user endpoints, and nothing else. The data and discovery
APIs have one owner each: `@objectstack/rest` and the runtime dispatcher
(ADR-0076 D11).

**Removed**

- `POST/GET /api/v1/data/:object` and `GET /api/v1/data/:object/:id` — the raw
C+R surface that delegated straight to ObjectQL.
- `GET /api/v1/discovery` and `GET /.well-known/objectstack` — this plugin's
third discovery payload, which predated `DiscoverySchema` and could not
satisfy it (no `services`, the ADR-0076 D12 source of truth).
- The `registerStandardEndpoints` option. It is gone, not defaulted off: passing
it is now a type error, and passing it via `as never` mounts nothing.

**Unaffected**

- `/auth/me/permissions`, `/auth/me/localization` and `/me/apps` — this plugin
is the platform's only supply and they register unconditionally (#4144).
- Every composed host: `os serve`, `objectstack dev`, cloud's objectos and every
documented composition mount REST and/or the dispatcher, which already served
these routes and answered byte-identically with the flag on or off (#4260).

**Migration** — only a host that mounts `HonoServerPlugin` with neither owner is
affected. It now has no data or discovery API, and the boot warns once naming
both remedies. Mount `createRestApiPlugin` from `@objectstack/rest` for full
CRUD behind the gate stack, or `createDispatcherPlugin` from
`@objectstack/runtime`. There is no flag to opt back in.

**Why** — the surface was duplicate and lesser supply (C+R only, a subset of the
gates, a non-conforming discovery payload), and it charged rent: #2567, #3298
and #4018 each had to re-implement a platform invariant on it after the fact,
because a second implementation of a route is a second place every future
invariant must be remembered.
41 changes: 41 additions & 0 deletions AGENTS.md
Original file line number Diff line number Diff line change
Expand Up @@ -365,6 +365,47 @@ export class MyPlugin implements Plugin {

---

## Route & surface ownership

Four rules, each paid for by a real bug. They matter more than usual here because
this repo is largely written by agents, and every one of them is a trap that
reads as reasonable code.

**1. One route, one owner.** Never add a second implementation of a path that
another package already serves, however convenient. A shadowed duplicate is code
that `grep` finds and the runtime never runs — the exact input that makes an
agent (or a human) reason confidently from dead code. It also silently forks
every future invariant: the retired hono `/data` surface had to re-learn the
anonymous-deny gate (#2567), honest batch capability reporting (#3298) and
discovery accuracy (#4018), each after the fact, each because someone fixed the
real owner and never knew about the copy. Retired in #4073.

**2. Explicit composition over default magic.** A capability that appears
because of a default nobody wrote down is invisible at every call site — and
call sites are the primary evidence an agent reasons from. #4073's own first
analysis checked *who passes the option* and missed *who relies on the default*;
the correction is in that issue's opening paragraph. If a host should get a
surface, it should mount it.

**3. Absence must be loud.** A composition that legitimately serves nothing
should say so once at boot, naming the remedy — never leave a bare 404 to be
diagnosed. The same rule applies to tooling: a verifier that silently degrades
(reusing a stale build, skipping a check it could not run) is worse than no
verifier, because it reports success. Prefer failing to falling back.

**4. Machine-readable surfaces must not lie.** `/discovery` and friends are read
by SDKs, codegen and AI clients. Advertise only what is actually mounted, and
mount everything advertised (ADR-0076 D12) — a wrong answer here propagates into
everything built on top of it.

**Verifying any of this:** "who serves this path" is a question about the
composed, *provisioned* runtime — not about which plugin declares it, not about
registration order, and not about a minimal harness that merely boots. #4073 was
answered wrongly three times, once per each of those shortcuts. Boot the real
composition with its real services, or do not claim an answer.

---

## Post-Task Checklist

1. `pnpm test` — verify nothing broke.
Expand Down
2 changes: 1 addition & 1 deletion content/docs/permissions/authorization.mdx
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ site — the file you read when behavior surprises you.

| # | Gate | What it decides | Enforcement site | Failure direction |
|---|---|---|---|---|
| 1 | **Anonymous deny** | No identity → HTTP 401. **Uniform across every HTTP surface that reaches object data** (#2567): REST `/data`, the metadata endpoints (`/meta`), and the raw-hono standard `/data` routes (the dispatcher GraphQL endpoint left the matrix when the GraphQL surface was removed in v17 — `/graphql` now 404s) — one shared decision, so a caller denied on `/data` can't read the same rows through a sibling door. **Default-on** (ADR-0056 D2): serving the whole data plane publicly requires an explicit `api.requireAuth: false` opt-out, which logs a boot warning. Narrower public surfaces do **not** need it — each derives its own authorization from a declaration rather than from the deployment posture: control plane (`/auth`, `/health`, `/discovery`) is allow-listed; public form submission carries a `publicFormGrant` (ADR-0056 Option A); share-links validate their token then read as SYSTEM; and an anonymous **GET** of the book/doc read surface is admitted so `book.audience: 'public'` works under the secure default, with the ADR-0046 §6.7 audience gate — `'public'` only, fail-closed — doing the authorizing (#3963). | `packages/core/src/security/anonymous-deny.ts` `shouldDenyAnonymous` — called by `rest-server.ts` `enforceAuth`, the dispatcher `handleMetadata`/`handleAI`, and `plugin-hono-server` `denyAnonymous` (default in `packages/spec/src/api/rest-server.zod.ts`); a source-enumerating ratchet in `authz-conformance.test.ts` fails CI if a new surface ships ungated | fail-closed |
| 1 | **Anonymous deny** | No identity → HTTP 401. **Uniform across every HTTP surface that reaches object data** (#2567): REST `/data` and the metadata endpoints (`/meta`) — the raw-hono standard `/data` routes left the matrix when that duplicate surface was deleted in v17 (#4073), and the dispatcher GraphQL endpoint left when the GraphQL surface was removed (`/graphql` now 404s) — one shared decision, so a caller denied on `/data` can't read the same rows through a sibling door. **Default-on** (ADR-0056 D2): serving the whole data plane publicly requires an explicit `api.requireAuth: false` opt-out, which logs a boot warning. Narrower public surfaces do **not** need it — each derives its own authorization from a declaration rather than from the deployment posture: control plane (`/auth`, `/health`, `/discovery`) is allow-listed; public form submission carries a `publicFormGrant` (ADR-0056 Option A); share-links validate their token then read as SYSTEM; and an anonymous **GET** of the book/doc read surface is admitted so `book.audience: 'public'` works under the secure default, with the ADR-0046 §6.7 audience gate — `'public'` only, fail-closed — doing the authorizing (#3963). | `packages/core/src/security/anonymous-deny.ts` `shouldDenyAnonymous` — called by `rest-server.ts` `enforceAuth` and the dispatcher `handleMetadata`/`handleAI` (default in `packages/spec/src/api/rest-server.zod.ts`); a source-enumerating ratchet in `authz-conformance.test.ts` fails CI if a new surface ships ungated | fail-closed |
| 2 | **Public-form grant** | An anonymous form submission carries a declaration-derived `publicFormGrant` authorizing ONLY create + read-back on the form's declared target object — never anything else (ADR-0056 Option A). No guest-portal configuration needed (anonymous principals hold the `guest` position). | `packages/plugins/plugin-security/src/security-plugin.ts` (ObjectQL middleware) | scope-limited allow |
| 3 | **Object CRUD** | `allowRead/Create/Edit/Delete` (+ the destructive lifecycle class `allowTransfer/Restore/Purge`, gated ahead of the M2 operations — #1883) resolved across the caller's permission sets. | `packages/plugins/plugin-security/src/permission-evaluator.ts` `checkObjectPermission` | fail-closed 403 |
| 4 | **OWD / sharing** | Org-wide default (`private` / `public_read` / `public_read_write` / `controlled_by_parent`; **unset or unknown ⇒ `private`, fail-closed** — ADR-0090 D1) plus the external dial (`externalSharingModel`, ADR-0090 D11), manual record shares, criteria sharing rules (owner-type rules were removed from the authoring surface in v17 rather than left declared-but-skipped — [Sharing Rules](/docs/permissions/sharing-rules#recipient-types)), business-unit hierarchy widening (ADR-0057 D5: scope-depth hierarchy lives on `sys_business_unit`, not positions). | `packages/plugins/plugin-sharing/src/sharing-service.ts` + `sharing-rule-service.ts` | fail-closed to owner-only |
Expand Down
1 change: 0 additions & 1 deletion packages/client/src/client.batch-transaction.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,6 @@ describe('data.batchTransaction (live Hono, #1604)', () => {
port: 0,
// Skip hardcoded hono CRUD routes so createRestApiPlugin owns
// route registration (including the root /batch route).
registerStandardEndpoints: false,
}),
);

Expand Down
1 change: 0 additions & 1 deletion packages/client/src/client.environment-scoping.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,6 @@ describe('Project-scoped REST routing (live Hono)', () => {
port: 0,
// IMPORTANT: skip hardcoded hono CRUD routes so createRestApiPlugin
// owns /data and /meta registration end-to-end.
registerStandardEndpoints: false,
});
kernel.use(honoPlugin);

Expand Down
45 changes: 22 additions & 23 deletions packages/client/src/client.hono.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ import { LiteKernel } from '@objectstack/core';
import { ObjectQL, ObjectQLPlugin } from '@objectstack/objectql';
import { SqliteWasmDriver } from '@objectstack/driver-sqlite-wasm';
import { HonoServerPlugin } from '@objectstack/plugin-hono-server';
import { createRestApiPlugin } from '@objectstack/runtime';
import { ObjectStackClient } from './index';

describe('ObjectStackClient (with Hono Server)', () => {
Expand All @@ -25,19 +26,22 @@ describe('ObjectStackClient (with Hono Server)', () => {
},
} as any);

// 2. Setup Hono Plugin
// This suite exercises the CLIENT's data operations over the raw-hono
// standard endpoints; it wires no auth service. Opt out of the
// secure-by-default anonymous-deny posture (#2567) so the anonymous
// data CRUD under test stays reachable — the same explicit
// `requireAuth: false` a deployment that intentionally serves data
// publicly would set. The gate itself is proven in
// plugin-hono-server/hono-anonymous-deny.test.ts.
const honoPlugin = new HonoServerPlugin({
port: 0,
registerStandardEndpoints: true,
});
// 2. Setup Hono Plugin — transport only.
//
// This suite used to run against the raw-hono standard endpoints. That
// surface is gone (#4073): the plugin serves the socket and the
// current-user endpoints, and the data + discovery APIs belong to their
// owner. What is under test here is the CLIENT — `connect()`, the
// discovery handshake, and CRUD over HTTP — so it now runs against the
// real owner, which is also what every deployment actually serves.
const honoPlugin = new HonoServerPlugin({ port: 0 });
kernel.use(honoPlugin);

// `requireAuth: false` keeps the anonymous CRUD under test reachable —
// the same explicit opt-out a deployment intentionally serving public
// data would set (#2567/#3963); the gate itself is proven in
// @objectstack/rest's own suites.
kernel.use(createRestApiPlugin({ api: { api: { requireAuth: false } as any } }));

// --- BROKER SHIM START ---
// HttpDispatcher requires a broker to function. We inject a simple shim.
Expand Down Expand Up @@ -152,19 +156,14 @@ describe('ObjectStackClient (with Hono Server)', () => {
// Client should have populated discovery info
expect(client['discoveryInfo']).toBeDefined();

// The standalone hono surface advertises what it actually mounts
// (#4018): /data CRUD and the /auth/me/* helpers are registered here,
// so both are advertised and both answer.
// Discovery is REST's, computed from its registry (#4018 D12: declared
// === enforced). Every route it advertises must actually answer.
const endpoints = client['discoveryInfo']!.routes;
expect(endpoints.data).toContain('/api/v1/data');
expect(endpoints.auth).toContain('/api/v1/auth');

// `metadata` is NOT advertised on this boot, and that is the point of
// #4018: no plugin here mounts /api/v1/meta (it ships with
// @objectstack/rest / the dispatcher), so the old hardcoded table was
// promising a route that 404s. Proof the omission is honest:
expect(endpoints.metadata).toBeUndefined();
expect((await fetch(`${baseUrl}/api/v1/meta/objects`)).status).toBe(404);
expect(endpoints.metadata).toContain('/api/v1/meta');

// Enforced, not just declared — the pairing #4018 exists to hold.
expect((await fetch(`${baseUrl}/api/v1/meta/objects`)).status).not.toBe(404);
});

it('should create and retrieve data via hono', async () => {
Expand Down
46 changes: 23 additions & 23 deletions packages/plugins/plugin-hono-server/src/current-user-endpoints.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,9 +13,10 @@
* `/auth/me/localization` for regional defaults, and `core`'s auth gate
* allow-lists `/me/apps` + `/me/localization` as endpoints a gated user MUST
* still reach to bootstrap the remediation UI. #4079 lifted them out from under
* `registerStandardEndpoints` (which covers only DUPLICATE supply — raw `/data`
* CRUD and a discovery the dispatcher/REST own) so the flag could not take the
* console down with it.
* `registerStandardEndpoints`, which also gated a DUPLICATE `/data` CRUD +
* discovery surface, so that flag could not take the console down with it. That
* surface and the flag have since been deleted (#4073); these three are all the
* plugin serves beyond the socket.
*
* That split fixed the flag but left the supply still welded to
* {@link HonoServerPlugin}: a host that stands up a bare {@link HonoHttpServer}
Expand Down Expand Up @@ -405,13 +406,14 @@ export function annotateEffectiveApiOperations(
}

/**
* Build the session → `ExecutionContext` resolver the current-user endpoints
* and the plugin's standalone `/data` CRUD surface — both need.
* Build the session → `ExecutionContext` resolver the current-user endpoints
* need.
*
* Extracted from `registerDiscoveryAndCrudEndpoints` when the current-user
* endpoints stopped being gated on `registerStandardEndpoints` (#4073): they
* resolve the same principal the `/data` routes do, and one resolver is the
* only way the two groups can agree on who the caller is.
* Extracted from `registerDiscoveryAndCrudEndpoints` when these endpoints
* stopped being gated on `registerStandardEndpoints` (#4073), so the two groups
* would agree on who the caller is. That surface has since been deleted and
* these endpoints are the only remaining caller — kept as a named export
* because the serverless host path (cloud#924) composes it directly.
*/
export function makeExecutionContextResolver(ctx: CurrentUserEndpointsContext) {
const getObjectQL = () => ctx.getService<IDataEngine>('objectql');
Expand Down Expand Up @@ -600,21 +602,19 @@ export function makeExecutionContextResolver(ctx: CurrentUserEndpointsContext) {
* Register the current-user endpoints — `/auth/me/permissions`,
* `/auth/me/localization` and `/me/apps` — on `rawApp`.
*
* When {@link HonoServerPlugin} drives this, it is UNCONDITIONAL, unlike the
* plugin's CRUD + discovery block (#4073). Those two groups used to ride on one
* `registerStandardEndpoints` flag, which conflated two opposite things. The flag
* covers DUPLICATE supply — raw `/data` CRUD that `@objectstack/rest` also serves
* (and, being registered first, really serves), plus a discovery that the
* dispatcher/REST own (#4018). These three are the opposite: nothing else in the
* platform mounts them. `packages/rest` and `packages/runtime` register no
* `/me/*` route at all, the console reads `/auth/me/permissions` for its whole
* permission layer and `/auth/me/localization` for regional defaults, and
* When {@link HonoServerPlugin} drives this, it is UNCONDITIONAL — and these are
* now the only routes it mounts beyond the socket itself.
*
* They used to ride on the `registerStandardEndpoints` flag alongside a raw
* `/data` CRUD + discovery surface, one flag over two opposite things (#4073).
* That surface was DUPLICATE supply and has been deleted; these three are the
* opposite and could not be. Nothing else in the platform mounts them:
* `packages/rest` and `packages/runtime` register no `/me/*` route at all, the
* console reads `/auth/me/permissions` for its whole permission layer and
* `/auth/me/localization` for regional defaults, and
* `core/security/auth-gate.ts` allow-lists `/me/apps` + `/me/localization` as
* endpoints a gated user MUST still reach to bootstrap the remediation UI.
* `os serve` gets them only because `registerStandardEndpoints` defaults to true
* (`cli/src/commands/serve.ts` passes just `{ port }`), so turning that flag off
* — or retiring the convenience surface it names — would have taken the console
* down with it.
* endpoints a gated user MUST still reach to bootstrap the remediation UI. The
* split (#4144) had to land before the deletion for exactly that reason.
*
* IDEMPOTENT: returns `false` and registers nothing when all three paths are
* already served. That is what lets a host call this eagerly on its own raw app
Expand Down
Loading
Loading