Skip to content

Commit 2053714

Browse files
authored
fix(hono,plugin-hono-server,runtime): one CORS source and one registry key — the last derivable copies from the #3786 sweep (#4282)
Re-ran the sweep across all 72 packages. The earlier pass globbed `packages/*/src`, one level deep, so it missed everything under `packages/plugins/` and `packages/adapters/`. A STALE CORS DEFAULT, on the one description callers actually read. HonoCorsOptions.allowHeaders' TSDoc promised ['Content-Type', 'Authorization', 'X-Requested-With'] "sufficient for cookie and bearer-token auth". The real default carries three more: X-Tenant-ID, X-Environment-Id (multi-tenant routing) and If-Match (OCC token on record PATCHes, objectui#2572). Sizing a custom allowHeaders against that sentence drops all three and breaks every cross-origin save. Three Hono CORS sites each kept their own copy under "keep in sync" comments. The copies agreed; the two prose descriptions each drifted separately — because the copies had each other to be diffed against and the prose had nothing. Both defaults are now single constants exported from plugin-hono-server and imported by the adapter (which already depends on it). The TSDoc links them instead of restating, and documents the asymmetry it never mentioned: allowHeaders REPLACES the default, exposeHeaders MERGES. A THIRD COPY, in the public protocol docs: http-protocol.mdx advertised "Access-Control-Allow-Headers: Authorization, Content-Type" — two of six — and methods missing PUT and HEAD, with no mention of exposed headers. That is the copy an integrator builds a client against: reading it you would not know If-Match is permitted, or that set-auth-token is readable. Corrected, with the non-obvious entries explained and a pointer to the constants. 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 fails `should allow If-Match by default`, by name. A HAND-COPIED REGISTRY KEY. runtime's share-links domain resolved 'shareLinks' as a literal, copied from SHARE_LINK_SERVICE — whose own comment says "keep in sync with the SharingPlugin registration". Now imports the constant. A drifted copy resolves nothing, so every share link 501s "Sharing is not configured for this environment" on an environment where it is. PLUS A DUPLICATE LEDGER ENTRY, 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. #4203's is kept. NOT included, because main already has it: wiring the reconciliation into CI. #4203 landed `check:generated --reconcile-only` in lint.yml's unfiltered job, strictly better than what this branch first proposed. Checked and left alone: ApprovalStatus (5 values) and ApprovalActionKind (12 values) vs their plugin-approvals selects — diffed verbatim, no drift today, still hand-copied across a package boundary. Verified: 28/28 turbo test tasks green (cache bypassed), 17/17 CI checks green.
1 parent 29e5a0e commit 2053714

8 files changed

Lines changed: 178 additions & 55 deletions

File tree

.changeset/sweep-close-out.md

Lines changed: 67 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,67 @@
1+
---
2+
"@objectstack/plugin-hono-server": patch
3+
"@objectstack/hono": patch
4+
"@objectstack/runtime": patch
5+
---
6+
7+
fix(hono,plugin-hono-server,runtime): one CORS source and one registry key — the last derivable copies from the #3786 sweep
8+
9+
Re-ran the sweep across all 72 packages. The earlier pass globbed `packages/*/src`,
10+
which is one level deep, so it missed everything under `packages/plugins/` and
11+
`packages/adapters/` — the "sweep is basically clean" report was based on an
12+
incomplete scan.
13+
14+
**A stale CORS default, on the one description callers actually read.**
15+
`HonoCorsOptions.allowHeaders`' TSDoc promised
16+
`['Content-Type', 'Authorization', 'X-Requested-With']` "which is sufficient for
17+
cookie and bearer-token auth". The real default carries three more:
18+
`X-Tenant-ID` and `X-Environment-Id` (multi-tenant routing) and `If-Match` (the
19+
OCC token on record PATCHes, objectui#2572). Sizing a custom `allowHeaders`
20+
against that sentence drops all three and every cross-origin save fails with
21+
"Failed to fetch".
22+
23+
The instructive part: **three** Hono CORS sites each carried their own copy of
24+
the defaults under "keep in sync" comments, and the copies all agreed. What
25+
drifted was the *doc* — the only description with no counterpart to be diffed
26+
against, and the only one a caller reads.
27+
28+
Both defaults are now single constants, `DEFAULT_CORS_ALLOW_HEADERS` and
29+
`DEFAULT_CORS_EXPOSE_HEADERS`, exported from `@objectstack/plugin-hono-server`
30+
and imported by the adapter (which already depends on it — no new edge). The
31+
TSDoc links them rather than restating, and documents an asymmetry it never
32+
mentioned: `allowHeaders` REPLACES the default, `exposeHeaders` MERGES with it.
33+
34+
`hono-plugin.test.ts` stopped stubbing `./adapter` wholesale and keeps the real
35+
constants via `importOriginal` — it asserts exact header lists, so a mocked copy
36+
would make the test agree with itself rather than with what ships. Verified:
37+
removing `If-Match` from the constant fails `should allow If-Match by default`,
38+
by name.
39+
40+
**A third copy, in the public protocol docs.** `content/docs/protocol/kernel/
41+
http-protocol.mdx` advertised `Access-Control-Allow-Headers: Authorization,
42+
Content-Type` — two of the six — and methods missing `PUT` and `HEAD`, with no
43+
mention of the exposed headers at all. That is the copy an integrator builds a
44+
client against: reading it, you would not know `If-Match` is permitted (so you
45+
would not attempt OCC) or that `set-auth-token` is readable (so a rotated
46+
session would look like a bug). Corrected, with the three non-obvious allowed
47+
headers and the two exposed ones explained, and a pointer to the constants as
48+
the source of truth.
49+
50+
**A hand-copied service-registry key.** `runtime`'s share-links domain resolved
51+
`'shareLinks'` as a string literal, copied from `SHARE_LINK_SERVICE` — whose own
52+
doc-comment says "keep in sync with the SharingPlugin registration". It now
53+
imports the constant. A drifted copy resolves nothing, so every share link
54+
answers 501 "Sharing is not configured for this environment" on an environment
55+
where it is configured perfectly well.
56+
57+
**Plus a duplicate ledger entry**, which is the same defect one level up:
58+
`check-generated.ts` carried two `NO_GENERATOR` entries for
59+
`check:strictness-ledger`, because #4203 and #4252 each added one without seeing
60+
the other. Functionally harmless (the ledger is read into a `Set`) but it leaves
61+
two comments telling overlapping versions of the same story. #4203's is kept —
62+
it is the more complete account and it is the PR that fixed the underlying
63+
problem.
64+
65+
Checked and deliberately left alone: `ApprovalStatus` (5 values) and
66+
`ApprovalActionKind` (12 values) versus their `plugin-approvals` selects — diffed
67+
verbatim, no drift today, still hand-copied across a package boundary.

content/docs/protocol/kernel/http-protocol.mdx

Lines changed: 22 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -791,11 +791,31 @@ ObjectStack sends CORS headers automatically:
791791

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

800+
Three of the allowed request headers are easy to overlook, and each one disables
801+
a feature if an intermediate proxy strips it:
802+
803+
| Header | Why it is allowed |
804+
| --- | --- |
805+
| `X-Tenant-ID` / `X-Environment-Id` | Route the request to its environment on a multi-tenant host. |
806+
| `If-Match` | Carries the OCC token on record `PATCH`es. Without it, a cross-origin save fails in the browser with "Failed to fetch". |
807+
808+
The two **exposed** response headers matter to browser clients specifically:
809+
`set-auth-token` delivers a rotated session token (without it a cross-origin
810+
session silently breaks even though every request succeeds), and
811+
`x-objectstack-dropped-fields` warns that a write dropped undeclared keys — the
812+
response body's `droppedFields` stays the primary channel for that.
813+
814+
These are the defaults exported as `DEFAULT_CORS_ALLOW_HEADERS` and
815+
`DEFAULT_CORS_EXPOSE_HEADERS` from `@objectstack/plugin-hono-server`. Supplying
816+
`allowHeaders` **replaces** the default; supplying `exposeHeaders` **merges**
817+
with it.
818+
799819
**Preflight request:**
800820
```http
801821
OPTIONS /api/v1/data/account

packages/adapters/hono/src/index.ts

Lines changed: 16 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -39,7 +39,12 @@ export type KernelManager = any;
3939
*/
4040
// eslint-disable-next-line @typescript-eslint/no-explicit-any
4141
export type EnvironmentDriverRegistry = any;
42-
import { createOriginMatcher, hasWildcardPattern } from '@objectstack/plugin-hono-server';
42+
import {
43+
createOriginMatcher,
44+
hasWildcardPattern,
45+
DEFAULT_CORS_ALLOW_HEADERS,
46+
DEFAULT_CORS_EXPOSE_HEADERS,
47+
} from '@objectstack/plugin-hono-server';
4348

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

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

193194
app.use('*', cors({
194195
origin: origin as any,
195196
allowMethods: corsOpts.methods || ['GET', 'POST', 'PUT', 'DELETE', 'PATCH', 'HEAD', 'OPTIONS'],
196-
// Keep in sync with plugin-hono-server's defaultAllowHeaders — `If-Match`
197-
// carries the OCC token on cross-origin record PATCHes (objectui#2572).
198-
allowHeaders: corsOpts.allowHeaders || ['Content-Type', 'Authorization', 'X-Requested-With', 'X-Tenant-ID', 'X-Environment-Id', 'If-Match'],
197+
allowHeaders: corsOpts.allowHeaders || [...DEFAULT_CORS_ALLOW_HEADERS],
199198
exposeHeaders,
200199
credentials,
201200
maxAge,

packages/plugins/plugin-hono-server/src/adapter.ts

Lines changed: 46 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -14,26 +14,64 @@ import { serve } from '@hono/node-server';
1414
import { serveStatic } from '@hono/node-server/serve-static';
1515
import { matchesRoutePattern } from './route-pattern';
1616

17+
/**
18+
* Request headers allowed on preflight, by default.
19+
*
20+
* **The** default — three Hono-based CORS sites apply it (this package's
21+
* `hono-plugin.ts` and the `@objectstack/hono` adapter, which depends on this
22+
* package), and they used to each carry their own copy under "keep in sync"
23+
* comments. The copies happened to agree; the TSDoc on {@link
24+
* HonoCorsOptions.allowHeaders} did not — it had been three headers behind for
25+
* long enough to predate multi-tenant routing, so the one description a caller
26+
* actually reads was the one that drifted (#3786).
27+
*
28+
* `X-Tenant-ID` / `X-Environment-Id` route a request to its environment.
29+
* `If-Match` carries the OCC token on record PATCHes (objectui's inline edit,
30+
* REST `update` with `ifMatch`) — without it in the preflight allow-list every
31+
* cross-origin save fails in the browser with "Failed to fetch" (objectui#2572).
32+
*/
33+
export const DEFAULT_CORS_ALLOW_HEADERS: readonly string[] = Object.freeze([
34+
'Content-Type',
35+
'Authorization',
36+
'X-Requested-With',
37+
'X-Tenant-ID',
38+
'X-Environment-Id',
39+
'If-Match',
40+
]);
41+
42+
/**
43+
* Response headers exposed to cross-origin JS, by default. Same three sites,
44+
* same reason as {@link DEFAULT_CORS_ALLOW_HEADERS}.
45+
*
46+
* `set-auth-token` lets better-auth's `bearer()` plugin hand rotated session
47+
* tokens to cross-origin clients (see plugin-auth). `x-objectstack-dropped-fields`
48+
* (#3455) exposes the single-write drop warning (#3431); the body `droppedFields`
49+
* channel remains the primary, cross-origin-safe surface.
50+
*/
51+
export const DEFAULT_CORS_EXPOSE_HEADERS: readonly string[] = Object.freeze([
52+
'set-auth-token',
53+
'x-objectstack-dropped-fields',
54+
]);
55+
1756
export interface HonoCorsOptions {
1857
enabled?: boolean;
1958
origins?: string | string[];
2059
methods?: string[];
2160
/**
2261
* Request headers allowed on preflight (`Access-Control-Allow-Headers`).
2362
*
24-
* Defaults to `['Content-Type', 'Authorization', 'X-Requested-With']`,
25-
* which is sufficient for cookie and bearer-token auth.
63+
* Defaults to {@link DEFAULT_CORS_ALLOW_HEADERS} — deliberately a link and
64+
* not a restatement. Supplying this REPLACES the default rather than
65+
* extending it, so spread the constant if you only mean to add:
66+
* `allowHeaders: [...DEFAULT_CORS_ALLOW_HEADERS, 'X-My-Header']`.
2667
*/
2768
allowHeaders?: string[];
2869
/**
2970
* Response headers exposed to JS (`Access-Control-Expose-Headers`).
3071
*
31-
* Defaults to `['set-auth-token', 'x-objectstack-dropped-fields']` so that
32-
* better-auth's `bearer()` plugin can hand rotated session tokens to
33-
* cross-origin clients, and a browser can read the single-write
34-
* `X-ObjectStack-Dropped-Fields` warning header (#3431/#3455). User-supplied
35-
* values are merged with these defaults — they are always exposed unless CORS
36-
* is disabled entirely.
72+
* Defaults to {@link DEFAULT_CORS_EXPOSE_HEADERS}. Unlike `allowHeaders`,
73+
* user-supplied values are MERGED with the default — those are always
74+
* exposed unless CORS is disabled entirely.
3775
*/
3876
exposeHeaders?: string[];
3977
credentials?: boolean;

packages/plugins/plugin-hono-server/src/hono-plugin.test.ts

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,13 @@ vi.mock('@hono/node-server/serve-static', () => ({
1515
serveStatic: vi.fn(() => (c: any, next: any) => next())
1616
}));
1717

18-
vi.mock('./adapter', () => ({
18+
// PARTIAL mock: only `HonoHttpServer` is replaced. The CORS default constants
19+
// are deliberately kept REAL via `importOriginal` — they are the single source
20+
// this plugin and the `@objectstack/hono` adapter both read (#3786), and the
21+
// assertions below check exact header lists, so stubbing them would make this
22+
// file agree with itself instead of with the shipped defaults.
23+
vi.mock('./adapter', async (importOriginal) => ({
24+
...(await importOriginal<typeof import('./adapter')>()),
1925
HonoHttpServer: vi.fn(function() {
2026
return {
2127
mount: vi.fn(),

packages/plugins/plugin-hono-server/src/hono-plugin.ts

Lines changed: 12 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,12 @@ import {
1212
makeExecutionContextResolver,
1313
registerCurrentUserEndpoints,
1414
} from './current-user-endpoints';
15-
import { HonoHttpServer, HonoCorsOptions } from './adapter';
15+
import {
16+
HonoHttpServer,
17+
HonoCorsOptions,
18+
DEFAULT_CORS_ALLOW_HEADERS,
19+
DEFAULT_CORS_EXPOSE_HEADERS,
20+
} from './adapter';
1621
import { cors } from 'hono/cors';
1722
import { serveStatic } from '@hono/node-server/serve-static';
1823
import * as fs from 'fs';
@@ -420,24 +425,13 @@ export class HonoServerPlugin implements Plugin {
420425
}
421426

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

packages/runtime/src/domains/share-links.ts

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,8 @@
3535
* value whichever one served the request.
3636
*/
3737

38+
import { SHARE_LINK_SERVICE } from '@objectstack/spec/contracts';
39+
3840
import type { HttpProtocolContext, HttpDispatcherResult } from '../http-dispatcher.js';
3941
import type { DomainHandlerDeps, DomainRoute } from '../domain-handler-registry.js';
4042

@@ -59,7 +61,12 @@ export async function handleShareLinksRequest(
5961
// [#4127 batch 3] `plugin-sharing` registers `ShareLinkService`, which
6062
// declares `implements IShareLinkService`; the four methods called below
6163
// were all already on that contract. Only the ledger entry was missing.
62-
const svc = await deps.resolveService('shareLinks', context.environmentId);
64+
// The registry key comes from the contract that DEFINES it (#3786). This was
65+
// a second hand-written `'shareLinks'`, copied from a constant whose own
66+
// doc-comment says "keep in sync with the SharingPlugin registration" — and a
67+
// drifted copy here resolves nothing, so every share link 501s with "Sharing
68+
// is not configured for this environment" on an environment where it is.
69+
const svc = await deps.resolveService(SHARE_LINK_SERVICE, context.environmentId);
6370
if (!svc) {
6471
return { handled: true, response: deps.error('Sharing is not configured for this environment', 501) };
6572
}

packages/spec/scripts/check-generated.ts

Lines changed: 0 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -85,14 +85,6 @@ const NO_GENERATOR: ReadonlyArray<{ check: string; why: string }> = [
8585
check: 'check:exported-any',
8686
why: 'audits the built .d.ts for exported types/schemas that resolve to `any` — no artifact (needs a fresh `pnpm build`)',
8787
},
88-
// Same cross-PR race as `check:variant-docs` above, now for the second time:
89-
// landed in #4232 with no entry here, so `main` again carries an unclassified
90-
// script and this reconciliation fails on `main` itself. The ledger it audits
91-
// is hand-written prose — there is no generator to name.
92-
{
93-
check: 'check:strictness-ledger',
94-
why: 'audits the hand-written #4001 strictness ledger against the z.object sites in src — no artifact',
95-
},
9688
];
9789

9890
/**

0 commit comments

Comments
 (0)