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
44 changes: 44 additions & 0 deletions .changeset/public-form-schema-declared-fields-only.md
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,44 @@
---
"@objectstack/rest": minor
---

fix(rest): 未声明字段的公开表单不再向匿名调用者发布目标对象的**全部**字段(#6601)

`GET /api/v1/forms/:slug` 会把目标对象的 schema 一并内嵌进应答,好让匿名前端不必再走
一次需要鉴权的 `/meta` 就能渲染表单。收窄的依据是表单 `sections` 声明的字段集合,但那段
代码写的是:

```ts
if (allowed.size === 0 || allowed.has(name)) { fields[name] = def; }
```

`allowed.size === 0` —— 表单**没有 sections**,或者 sections 一个字段都没声明 —— 会
落到「发布该对象每一个非 server-managed 字段」这一支。**这条路由是匿名的**,所以发出去的
是完整的字段定义:label、type、picklist 的选项值(常常就是一份运营分类表)、formula
表达式(定价/评分 IP)。下方的 `safeForm` 只过滤表单自己的 `sections`(未声明
`publicPicker` 的 lookup),它与 `objectSchema.fields` 是同一份应答上的两个并列键,从不
收窄后者。那段代码上方注释里的「limited to fields referenced by the form」在这一支上是
不成立的;注释同时提到的「submit 侧仍有服务端字段白名单」是**写**侧防线,挡不住**读**侧
的披露。

「表单先建、sections 之后再配」是完全正常的编写中间态,所以这不是一个刁钻配置。
ADR-0106(#3682)刚刚让平台能完整地讲出「调用者读不到的字段,对它而言在任何平面上都不
存在」这句话,而这条路由是它剩下的那个反例,且调用者是**匿名**的。

**行为变化(线上可见)。** 发布集合现在等于表单声明的字段集合本身:

```ts
if (!allowed.has(name)) continue;
```

一个字段都没声明的表单,`objectSchema.fields` 就是 `{}`。应答的信封形状不变
(`objectSchema` 仍是 `{ name, label, fields }`,不会变成 `null`),`object` /
`label` / `form` 几个键也都不变。**已经正常声明了 sections 的表单,应答逐字节不变** ——
它们本来走的就是 `allowed.has(name)` 那一支。

这里没有新增任何可编写的键。发布应当是一次**声明**,而不是从空集合里掉出来的默认值
(AGENTS.md「Explicit composition over default magic」);真需要「整对象发布」的场景,
带着真实用例来提,再按 ADR-0049「没有需求牵引就不造能力」的顺序决定要不要造这个开关。

`PUBLIC_FORM_SERVER_MANAGED_FIELDS` 的处理(#3022 的 server-managed 锚点)完全未动,
`POST /forms/:slug/submit` 与 `GET /forms/:slug/lookup/:field` 也都未动。
117 changes: 108 additions & 9 deletions packages/rest/src/public-form-routes.test.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -28,7 +28,7 @@ function mockRes() {
}

/** A public FormView in the flattened registered shape (one item per view). */
function formView(sections: any[]) {
function formView(sections: any[] | undefined) {
return {
name: 'ticket_form',
object: 'ticket',
Expand All@@ -49,14 +49,32 @@ const ticketObject = {
subject: { type: 'text', label: 'Subject' },
email: { type: 'text', label: 'Email' },
status: { type: 'select', label: 'Status' },
// [#6601] Two fields an anonymous caller must not learn about, standing in
// for what ADR-0106's Context section names: a picklist whose OPTION VALUES
// are an operational taxonomy, and a formula whose EXPRESSION is pricing IP.
// Neither is a server-managed anchor, so #3022's set does not cover them —
// only the declared-field narrowing does.
internal_tier: {
type: 'select',
label: 'Internal Tier',
options: [
{ label: 'Strategic', value: 'strategic' },
{ label: 'Churn Risk', value: 'churn_risk' },
],
},
internal_margin: {
type: 'formula',
label: 'Margin',
formula: '(amount - cost) / amount * 0.87',
},
owner_id: { type: 'lookup', reference: 'sys_user', label: 'Owner' },
organization_id: { type: 'lookup', reference: 'sys_organization', label: 'Organization' },
created_at: { type: 'datetime', label: 'Created At' },
created_by: { type: 'lookup', reference: 'sys_user', label: 'Created By' },
},
};

function buildServer(sections: any[]) {
function buildServer(sections: any[] | undefined) {
const createData = vi.fn().mockResolvedValue({ object: 'ticket', id: 'rec_1', record: {} });
const protocol: any = {
getDiscovery: vi.fn().mockResolvedValue({ version: 'v0', routes: { data: '', metadata: '' } }),
Expand DownExpand Up@@ -142,13 +160,15 @@ describe('POST /forms/:slug/submit — server-managed anchors (#3022)', () => {
});

describe('GET /forms/:slug — schema/sections agree with the submit boundary (#3022)', () => {
it('zero declared sections: the all-fields schema expansion excludes managed anchors', async () => {
const { resolve } = buildServer([]);
const res = mockRes();
await resolve.handler({ params: { slug: 'test' }, headers: {} } as any, res);
expect(res.statusCode).toBe(200);
expect(Object.keys(res.body.objectSchema.fields).sort()).toEqual(['email', 'status', 'subject']);
});
// The zero-sections case used to live here as "the all-fields schema
// expansion excludes managed anchors", asserting
// `['email', 'status', 'subject']`. That test pinned the ALL-FIELDS
// expansion as correct and only checked that #3022's anchors stayed out of
// it. #6601 removed the expansion outright, so the anchor property is vacuous
// on that path (nothing at all is published) and the case now lives in the
// #6601 block below, asserting the empty set. No coverage was dropped: the
// anchor property still has its own zero-sections pin on the SUBMIT route
// above, which is where it does work.

it('a declared owner_id is dropped from the rendered sections and schema', async () => {
const { resolve } = buildServer([{ fields: ['subject', { field: 'owner_id' }] }]);
Expand All@@ -162,6 +182,85 @@ describe('GET /forms/:slug — schema/sections agree with the submit boundary (#
});
});

describe('GET /forms/:slug — the published schema IS the declared field set (#6601)', () => {
// Before this pin the narrowing read `allowed.size === 0 || allowed.has(name)`.
// A form that declared no fields therefore fell through to EVERY
// non-server-managed field of the target object, published to an ANONYMOUS
// caller: labels, types, `internal_tier`'s picklist OPTION VALUES and
// `internal_margin`'s formula EXPRESSION. A form created before its sections
// are wired is a plausible authoring mid-state, so nothing exotic was needed
// to reach it, and the route comment's "limited to fields referenced by the
// form" was simply false there.
//
// Why the fix is "declare it or it is not published" rather than "publish
// what the submit route accepts": the submit route's accepted set degenerates
// the SAME way for a section-less form (`allowedFields.size === 0 ||`, pinned
// by 'zero declared sections: business fields fall through' above, which
// shows an undeclared `status` being accepted). Aligning the read surface to
// that write surface would have republished exactly the set we are removing.
// The submit-side whitelist is a WRITE control and never was the backstop for
// a READ disclosure.

const SENSITIVE = ['internal_margin', 'internal_tier'];

async function publishedFields(sections: any[] | undefined) {
const { resolve } = buildServer(sections);
const res = mockRes();
await resolve.handler({ params: { slug: 'test' }, headers: {} } as any, res);
expect(res.statusCode).toBe(200);
return res;
}

it('zero declared sections: the schema publishes NOTHING, not every field of the object', async () => {
const res = await publishedFields([]);
expect(Object.keys(res.body.objectSchema.fields)).toEqual([]);
// The envelope shape is unchanged — `objectSchema` stays an object, so no
// client has to learn a new `null` case for this response.
expect(res.body.objectSchema).toMatchObject({ name: 'ticket', label: 'Ticket' });
});

it('zero declared sections: option values and formula expressions are not on the wire at all', async () => {
const res = await publishedFields([]);
for (const name of SENSITIVE) {
expect(res.body.objectSchema.fields[name], `${name} must not be published`).toBeUndefined();
}
// Asserted against the whole serialized response, not just the key we
// happened to look under: the finding is about the VALUES escaping.
const wire = JSON.stringify(res.body);
expect(wire, 'picklist option value leaked').not.toContain('churn_risk');
expect(wire, 'formula expression leaked').not.toContain('amount - cost');
});

it('sections that exist but declare no fields publish nothing either', async () => {
const res = await publishedFields([{ label: 'Details', fields: [] }, { label: 'More' }]);
expect(Object.keys(res.body.objectSchema.fields)).toEqual([]);
});

it('`sections` omitted entirely publishes nothing', async () => {
const res = await publishedFields(undefined);
expect(Object.keys(res.body.objectSchema.fields)).toEqual([]);
});

it('NO-REGRESSION: a form that declares sections publishes exactly those fields, as it always did', async () => {
// This assertion held BEFORE the change too — the declared path always took
// `allowed.has(name)`. It is a guard that working forms did not break, not
// evidence for the fix.
const res = await publishedFields([{ fields: ['subject', { field: 'email' }] }]);
expect(Object.keys(res.body.objectSchema.fields).sort()).toEqual(['email', 'subject']);
for (const name of SENSITIVE) {
expect(res.body.objectSchema.fields[name]).toBeUndefined();
}
});

it('NO-REGRESSION: a declared field publishes its full definition, options included', async () => {
// The narrowing decides WHICH fields are published, never how much of one:
// an author who declares `internal_tier` on a public form is publishing it.
const res = await publishedFields([{ fields: ['internal_tier'] }]);
expect(Object.keys(res.body.objectSchema.fields)).toEqual(['internal_tier']);
expect(res.body.objectSchema.fields.internal_tier.options).toHaveLength(2);
});
});

describe('GET /forms/:slug/lookup/:field — no picker on managed anchors (#3022)', () => {
it('refuses a publicPicker declared on owner_id (would open anonymous sys_user search)', async () => {
const { lookup } = buildServer([
Expand Down
42 changes: 32 additions & 10 deletions packages/rest/src/rest-server.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -7090,11 +7090,32 @@ export class RestServer {
});
return;
}
// Embed the target object's schema (limited to fields
// referenced by the form) so anonymous front-ends can
// render the form without a separate, auth-protected
// meta lookup. The submit handler still enforces the
// field whitelist server-side.
// Embed the target object's schema — limited to exactly the
// fields the form's sections DECLARE — so anonymous
// front-ends can render the form without a separate,
// auth-protected meta lookup.
//
// [#6601] "Exactly" is load-bearing and used not to be. The
// narrowing read `allowed.size === 0 || allowed.has(name)`,
// so a form with no sections (or sections declaring no
// fields) fell through to EVERY non-server-managed field of
// the object — published to an ANONYMOUS caller, with
// labels, types, picklist option values and formula
// expressions. A form created before its sections are wired
// is an ordinary authoring mid-state, so that was reachable
// without an exotic configuration, and this comment claimed
// the opposite. Publication is a DECLARATION now: declare no
// fields and nothing is published (AGENTS.md "Explicit
// composition over default magic").
//
// Do NOT reach for the submit handler as the backstop here.
// It enforces a field whitelist on WRITES, which cannot
// bound a READ disclosure — and its own accepted set
// degenerates identically for a section-less form
// (`allowedFields.size === 0 ||` below), so narrowing this
// schema to "what submit would accept" would have
// republished precisely the set being removed. That
// write-side twin is tracked separately in #6920.
let objectSchema: any = null;
try {
const p = await this.resolveProtocol(environmentId, req);
Expand All@@ -7118,12 +7139,13 @@ export class RestServer {
// [#3022] Server-managed anchors are never
// renderable/writable on the anonymous form
// surface — the submit route refuses them, so
// don't advertise them here (declared or via
// the zero-sections all-fields expansion).
// don't advertise them here even when a form
// (mis)declares one in a section.
if (PUBLIC_FORM_SERVER_MANAGED_FIELDS.has(name)) continue;
if (allowed.size === 0 || allowed.has(name)) {
fields[name] = def;
}
// [#6601] Declared or not published. An empty
// `allowed` yields an empty `fields`.
if (!allowed.has(name)) continue;
fields[name] = def;
}
objectSchema = { name: obj.name, label: obj.label, fields };
// Localize labels / help text / option labels so anonymous
Expand Down
Loading