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
67 changes: 67 additions & 0 deletions .changeset/sweep-close-out.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,67 @@
---
"@objectstack/plugin-hono-server": patch
"@objectstack/hono": patch
"@objectstack/runtime": patch
---

fix(hono,plugin-hono-server,runtime): one CORS source and one registry key — the last derivable copies from the #3786 sweep

Re-ran the sweep across all 72 packages. The earlier pass globbed `packages/*/src`,
which is one level deep, so it missed everything under `packages/plugins/` and
`packages/adapters/` — the "sweep is basically clean" report was based on an
incomplete scan.

**A stale CORS default, on the one description callers actually read.**
`HonoCorsOptions.allowHeaders`' TSDoc promised
`['Content-Type', 'Authorization', 'X-Requested-With']` "which is sufficient for
cookie and bearer-token auth". The real default carries three more:
`X-Tenant-ID` and `X-Environment-Id` (multi-tenant routing) and `If-Match` (the
OCC token on record PATCHes, objectui#2572). Sizing a custom `allowHeaders`
against that sentence drops all three and every cross-origin save fails with
"Failed to fetch".

The instructive part: **three** Hono CORS sites each carried their own copy of
the defaults under "keep in sync" comments, and the copies all agreed. What
drifted was the *doc* — the only description with no counterpart to be diffed
against, and the only one a caller reads.

Both defaults are now single constants, `DEFAULT_CORS_ALLOW_HEADERS` and
`DEFAULT_CORS_EXPOSE_HEADERS`, exported from `@objectstack/plugin-hono-server`
and imported by the adapter (which already depends on it — no new edge). The
TSDoc links them rather than restating, and documents an asymmetry it never
mentioned: `allowHeaders` REPLACES the default, `exposeHeaders` MERGES with it.

`hono-plugin.test.ts` stopped stubbing `./adapter` wholesale and keeps the real
constants via `importOriginal` — it asserts exact header lists, so a mocked copy
would make the test agree with itself rather than with what ships. Verified:
removing `If-Match` from the constant fails `should allow If-Match by default`,
by name.

**A third copy, in the public protocol docs.** `content/docs/protocol/kernel/
http-protocol.mdx` advertised `Access-Control-Allow-Headers: Authorization,
Content-Type` — two of the six — and methods missing `PUT` and `HEAD`, with no
mention of the exposed headers at all. That is the copy an integrator builds a
client against: reading it, you would not know `If-Match` is permitted (so you
would not attempt OCC) or that `set-auth-token` is readable (so a rotated
session would look like a bug). Corrected, with the three non-obvious allowed
headers and the two exposed ones explained, and a pointer to the constants as
the source of truth.

**A hand-copied service-registry key.** `runtime`'s share-links domain resolved
`'shareLinks'` as a string literal, copied from `SHARE_LINK_SERVICE` — whose own
doc-comment says "keep in sync with the SharingPlugin registration". It now
imports the constant. A drifted copy resolves nothing, so every share link
answers 501 "Sharing is not configured for this environment" on an environment
where it is configured perfectly well.

**Plus a duplicate ledger entry**, which is the same defect one level up:
`check-generated.ts` carried two `NO_GENERATOR` entries for
`check:strictness-ledger`, because #4203 and #4252 each added one without seeing
the other. Functionally harmless (the ledger is read into a `Set`) but it leaves
two comments telling overlapping versions of the same story. #4203's is kept —
it is the more complete account and it is the PR that fixed the underlying
problem.

Checked and deliberately left alone: `ApprovalStatus` (5 values) and
`ApprovalActionKind` (12 values) versus their `plugin-approvals` selects — diffed
verbatim, no drift today, still hand-copied across a package boundary.
24 changes: 22 additions & 2 deletions content/docs/protocol/kernel/http-protocol.mdx
Original file line number Diff line number Diff line change
Expand Up @@ -791,11 +791,31 @@ ObjectStack sends CORS headers automatically:

```http
Access-Control-Allow-Origin: https://app.acme.com
Access-Control-Allow-Methods: GET, POST, PATCH, DELETE, OPTIONS
Access-Control-Allow-Headers: Authorization, Content-Type
Access-Control-Allow-Methods: GET, POST, PUT, DELETE, PATCH, HEAD, OPTIONS
Access-Control-Allow-Headers: Content-Type, Authorization, X-Requested-With, X-Tenant-ID, X-Environment-Id, If-Match
Access-Control-Expose-Headers: set-auth-token, x-objectstack-dropped-fields
Access-Control-Max-Age: 86400
```

Three of the allowed request headers are easy to overlook, and each one disables
a feature if an intermediate proxy strips it:

| Header | Why it is allowed |
| --- | --- |
| `X-Tenant-ID` / `X-Environment-Id` | Route the request to its environment on a multi-tenant host. |
| `If-Match` | Carries the OCC token on record `PATCH`es. Without it, a cross-origin save fails in the browser with "Failed to fetch". |

The two **exposed** response headers matter to browser clients specifically:
`set-auth-token` delivers a rotated session token (without it a cross-origin
session silently breaks even though every request succeeds), and
`x-objectstack-dropped-fields` warns that a write dropped undeclared keys — the
response body's `droppedFields` stays the primary channel for that.

These are the defaults exported as `DEFAULT_CORS_ALLOW_HEADERS` and
`DEFAULT_CORS_EXPOSE_HEADERS` from `@objectstack/plugin-hono-server`. Supplying
`allowHeaders` **replaces** the default; supplying `exposeHeaders` **merges**
with it.

**Preflight request:**
```http
OPTIONS /api/v1/data/account
Expand Down
33 changes: 16 additions & 17 deletions packages/adapters/hono/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,12 @@ export type KernelManager = any;
*/
// eslint-disable-next-line @typescript-eslint/no-explicit-any
export type EnvironmentDriverRegistry = any;
import { createOriginMatcher, hasWildcardPattern } from '@objectstack/plugin-hono-server';
import {
createOriginMatcher,
hasWildcardPattern,
DEFAULT_CORS_ALLOW_HEADERS,
DEFAULT_CORS_EXPOSE_HEADERS,
} from '@objectstack/plugin-hono-server';

export interface ObjectStackHonoCorsOptions {
/** Enable or disable CORS. Defaults to true. */
Expand Down Expand Up @@ -172,30 +177,24 @@ export function createHonoApp(options: ObjectStackHonoOptions): Hono {
origin = configuredOrigin;
}

// Always include `set-auth-token` in exposed headers so that the
// better-auth `bearer()` plugin (registered by plugin-auth) can
// deliver rotated session tokens to cross-origin clients. Without
// this, browsers strip the header from every response, the client
// never sees the new token, and cross-origin sessions silently
// break even when preflight and the actual request both succeed.
// Both CORS defaults are imported from `@objectstack/plugin-hono-server`,
// which this package already depends on (#3786). The three Hono-based CORS
// sites used to carry their own copies under "keep in sync" comments — this
// one included, right down to a duplicate of the rationale below.
//
// This mirrors `plugin-hono-server`'s CORS wiring — all three
// Hono-based CORS sites must stay in lockstep on this default.
// `x-objectstack-dropped-fields` (#3455) lets a cross-origin browser read
// the single-write drop header (#3431); the body `droppedFields` channel is
// the primary, cross-origin-safe surface, so this is a convenience mirror.
const defaultExposeHeaders = ['set-auth-token', 'x-objectstack-dropped-fields'];
// `set-auth-token` is the load-bearing one to understand: without it in the
// exposed set, browsers strip the header from every response, the client
// never sees its rotated session token, and cross-origin sessions silently
// break even though preflight and the request both succeed.
const exposeHeaders = Array.from(new Set([
...defaultExposeHeaders,
...DEFAULT_CORS_EXPOSE_HEADERS,
...(corsOpts.exposeHeaders ?? []),
]));

app.use('*', cors({
origin: origin as any,
allowMethods: corsOpts.methods || ['GET', 'POST', 'PUT', 'DELETE', 'PATCH', 'HEAD', 'OPTIONS'],
// Keep in sync with plugin-hono-server's defaultAllowHeaders — `If-Match`
// carries the OCC token on cross-origin record PATCHes (objectui#2572).
allowHeaders: corsOpts.allowHeaders || ['Content-Type', 'Authorization', 'X-Requested-With', 'X-Tenant-ID', 'X-Environment-Id', 'If-Match'],
allowHeaders: corsOpts.allowHeaders || [...DEFAULT_CORS_ALLOW_HEADERS],
exposeHeaders,
credentials,
maxAge,
Expand Down
54 changes: 46 additions & 8 deletions packages/plugins/plugin-hono-server/src/adapter.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,26 +14,64 @@ import { serve } from '@hono/node-server';
import { serveStatic } from '@hono/node-server/serve-static';
import { matchesRoutePattern } from './route-pattern';

/**
* Request headers allowed on preflight, by default.
*
* **The** default — three Hono-based CORS sites apply it (this package's
* `hono-plugin.ts` and the `@objectstack/hono` adapter, which depends on this
* package), and they used to each carry their own copy under "keep in sync"
* comments. The copies happened to agree; the TSDoc on {@link
* HonoCorsOptions.allowHeaders} did not — it had been three headers behind for
* long enough to predate multi-tenant routing, so the one description a caller
* actually reads was the one that drifted (#3786).
*
* `X-Tenant-ID` / `X-Environment-Id` route a request to its environment.
* `If-Match` carries the OCC token on record PATCHes (objectui's inline edit,
* REST `update` with `ifMatch`) — without it in the preflight allow-list every
* cross-origin save fails in the browser with "Failed to fetch" (objectui#2572).
*/
export const DEFAULT_CORS_ALLOW_HEADERS: readonly string[] = Object.freeze([
'Content-Type',
'Authorization',
'X-Requested-With',
'X-Tenant-ID',
'X-Environment-Id',
'If-Match',
]);

/**
* Response headers exposed to cross-origin JS, by default. Same three sites,
* same reason as {@link DEFAULT_CORS_ALLOW_HEADERS}.
*
* `set-auth-token` lets better-auth's `bearer()` plugin hand rotated session
* tokens to cross-origin clients (see plugin-auth). `x-objectstack-dropped-fields`
* (#3455) exposes the single-write drop warning (#3431); the body `droppedFields`
* channel remains the primary, cross-origin-safe surface.
*/
export const DEFAULT_CORS_EXPOSE_HEADERS: readonly string[] = Object.freeze([
'set-auth-token',
'x-objectstack-dropped-fields',
]);

export interface HonoCorsOptions {
enabled?: boolean;
origins?: string | string[];
methods?: string[];
/**
* Request headers allowed on preflight (`Access-Control-Allow-Headers`).
*
* Defaults to `['Content-Type', 'Authorization', 'X-Requested-With']`,
* which is sufficient for cookie and bearer-token auth.
* Defaults to {@link DEFAULT_CORS_ALLOW_HEADERS} — deliberately a link and
* not a restatement. Supplying this REPLACES the default rather than
* extending it, so spread the constant if you only mean to add:
* `allowHeaders: [...DEFAULT_CORS_ALLOW_HEADERS, 'X-My-Header']`.
*/
allowHeaders?: string[];
/**
* Response headers exposed to JS (`Access-Control-Expose-Headers`).
*
* Defaults to `['set-auth-token', 'x-objectstack-dropped-fields']` so that
* better-auth's `bearer()` plugin can hand rotated session tokens to
* cross-origin clients, and a browser can read the single-write
* `X-ObjectStack-Dropped-Fields` warning header (#3431/#3455). User-supplied
* values are merged with these defaults — they are always exposed unless CORS
* is disabled entirely.
* Defaults to {@link DEFAULT_CORS_EXPOSE_HEADERS}. Unlike `allowHeaders`,
* user-supplied values are MERGED with the default — those are always
* exposed unless CORS is disabled entirely.
*/
exposeHeaders?: string[];
credentials?: boolean;
Expand Down
8 changes: 7 additions & 1 deletion packages/plugins/plugin-hono-server/src/hono-plugin.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,13 @@ vi.mock('@hono/node-server/serve-static', () => ({
serveStatic: vi.fn(() => (c: any, next: any) => next())
}));

vi.mock('./adapter', () => ({
// PARTIAL mock: only `HonoHttpServer` is replaced. The CORS default constants
// are deliberately kept REAL via `importOriginal` — they are the single source
// this plugin and the `@objectstack/hono` adapter both read (#3786), and the
// assertions below check exact header lists, so stubbing them would make this
// file agree with itself instead of with the shipped defaults.
vi.mock('./adapter', async (importOriginal) => ({
...(await importOriginal<typeof import('./adapter')>()),
HonoHttpServer: vi.fn(function() {
return {
mount: vi.fn(),
Expand Down
30 changes: 12 additions & 18 deletions packages/plugins/plugin-hono-server/src/hono-plugin.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,12 @@ import {
makeExecutionContextResolver,
registerCurrentUserEndpoints,
} from './current-user-endpoints';
import { HonoHttpServer, HonoCorsOptions } from './adapter';
import {
HonoHttpServer,
HonoCorsOptions,
DEFAULT_CORS_ALLOW_HEADERS,
DEFAULT_CORS_EXPOSE_HEADERS,
} from './adapter';
import { cors } from 'hono/cors';
import { serveStatic } from '@hono/node-server/serve-static';
import * as fs from 'fs';
Expand Down Expand Up @@ -420,24 +425,13 @@ export class HonoServerPlugin implements Plugin {
}

const rawApp = this.server.getRawApp();
// Always include `set-auth-token` in exposed headers so that
// the better-auth `bearer()` plugin can deliver rotated
// session tokens to cross-origin clients (see plugin-auth).
// User-supplied exposeHeaders are merged with this default.
// `If-Match` carries the OCC token on record PATCHes (objectui's
// record-level inline edit, REST `update` with `ifMatch`) — without
// it in the preflight allow-list, every cross-origin save fails in
// the browser with "Failed to fetch" (objectui#2572 dogfood find;
// same split-origin class as the #2548 Bearer fixes).
const defaultAllowHeaders = ['Content-Type', 'Authorization', 'X-Requested-With', 'X-Tenant-ID', 'X-Environment-Id', 'If-Match'];
// `x-objectstack-dropped-fields` (#3455): expose the single-write
// drop header (#3431) to cross-origin JS. Kept in lockstep with the
// `@objectstack/hono` adapter's default. The body `droppedFields`
// channel remains the primary, cross-origin-safe surface.
const defaultExposeHeaders = ['set-auth-token', 'x-objectstack-dropped-fields'];
const allowHeaders = corsOpts.allowHeaders ?? defaultAllowHeaders;
// Both defaults come from `adapter.ts` (#3786). They used to be
// spelled out here AND in the `@objectstack/hono` adapter, each
// under a "keep in sync" comment; the per-entry rationale now
// lives on the constants themselves.
const allowHeaders = corsOpts.allowHeaders ?? [...DEFAULT_CORS_ALLOW_HEADERS];
const exposeHeaders = Array.from(new Set([
...defaultExposeHeaders,
...DEFAULT_CORS_EXPOSE_HEADERS,
...(corsOpts.exposeHeaders ?? []),
]));

Expand Down
9 changes: 8 additions & 1 deletion packages/runtime/src/domains/share-links.ts
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,8 @@
* value whichever one served the request.
*/

import { SHARE_LINK_SERVICE } from '@objectstack/spec/contracts';

import type { HttpProtocolContext, HttpDispatcherResult } from '../http-dispatcher.js';
import type { DomainHandlerDeps, DomainRoute } from '../domain-handler-registry.js';

Expand All @@ -59,7 +61,12 @@ export async function handleShareLinksRequest(
// [#4127 batch 3] `plugin-sharing` registers `ShareLinkService`, which
// declares `implements IShareLinkService`; the four methods called below
// were all already on that contract. Only the ledger entry was missing.
const svc = await deps.resolveService('shareLinks', context.environmentId);
// The registry key comes from the contract that DEFINES it (#3786). This was
// a second hand-written `'shareLinks'`, copied from a constant whose own
// doc-comment says "keep in sync with the SharingPlugin registration" — and a
// drifted copy here resolves nothing, so every share link 501s with "Sharing
// is not configured for this environment" on an environment where it is.
const svc = await deps.resolveService(SHARE_LINK_SERVICE, context.environmentId);
if (!svc) {
return { handled: true, response: deps.error('Sharing is not configured for this environment', 501) };
}
Expand Down
8 changes: 0 additions & 8 deletions packages/spec/scripts/check-generated.ts
Original file line number Diff line number Diff line change
Expand Up @@ -85,14 +85,6 @@ const NO_GENERATOR: ReadonlyArray<{ check: string; why: string }> = [
check: 'check:exported-any',
why: 'audits the built .d.ts for exported types/schemas that resolve to `any` — no artifact (needs a fresh `pnpm build`)',
},
// Same cross-PR race as `check:variant-docs` above, now for the second time:
// landed in #4232 with no entry here, so `main` again carries an unclassified
// script and this reconciliation fails on `main` itself. The ledger it audits
// is hand-written prose — there is no generator to name.
{
check: 'check:strictness-ledger',
why: 'audits the hand-written #4001 strictness ledger against the z.object sites in src — no artifact',
},
];

/**
Expand Down
Loading