Skip to content

Commit aaec5db

Browse files
authored
fix(security): public-form submissions can no longer forge server-managed anchors (#3022) (#3036)
Anonymous public-form submits (ADR-0056 Option A publicFormGrant) bypassed every write gate; a FormView with zero declared section fields fell back to merging the raw body wholesale, letting an unauthenticated visitor forge owner_id / organization_id / audit columns. Enforce the server-managed anchor set at both the REST route layer and the data-layer grant branch from one shared spec definition (PUBLIC_FORM_SERVER_MANAGED_FIELDS), refuse publicPicker on managed anchors, and block __proto__ prototype smuggling. Closes #3022.
1 parent d687f08 commit aaec5db

11 files changed

Lines changed: 440 additions & 15 deletions

File tree

Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,42 @@
1+
---
2+
'@objectstack/plugin-security': patch
3+
'@objectstack/rest': patch
4+
'@objectstack/spec': patch
5+
---
6+
7+
fix(security): public-form submissions can no longer forge server-managed anchors (#3022)
8+
9+
The anonymous public-form surface (ADR-0056 Option A, `POST /forms/:slug/submit`)
10+
is authorized by the declaration-derived `publicFormGrant`, which short-circuits
11+
the security middleware BEFORE every write gate (CRUD, FLS, the owner anchor
12+
guard, the tenant CHECK). The only field-side defense was the route's
13+
declared-field allow-list — and a FormView with zero declared section fields
14+
fell back to merging the raw body wholesale, so an unauthenticated visitor
15+
could `POST owner_id=<victim>` (or `organization_id`, audit columns, `id`) and
16+
attach the record to another user or tenant — the #3004 insert-forge, with no
17+
credentials at all.
18+
19+
Server-managed anchors are now enforced on this surface at BOTH layers, from a
20+
single shared definition (`PUBLIC_FORM_SERVER_MANAGED_FIELDS`, new in
21+
`@objectstack/spec/security`):
22+
23+
- **Data layer (authoritative)** — the `publicFormGrant` branch in
24+
`@objectstack/plugin-security` strips `id` / `owner_id` / `organization_id` /
25+
`tenant_id` / audit columns / soft-delete state / `__search` from every row
26+
of a granted insert (batch included) before admitting the write, so the
27+
boundary holds no matter what any route lets through. Ownership stays NULL
28+
for object hooks / the first-admin bootstrap to assign, as for other
29+
anonymous-seeded rows.
30+
- **Route layer** — the submit allow-list excludes the same set
31+
unconditionally: an explicitly declared `owner_id` section field no longer
32+
passes, and the zero-declared-sections fallback keeps its documented
33+
all-fields behavior for business columns while refusing the managed set.
34+
The resolve route (`GET /forms/:slug`) drops the managed fields from the
35+
rendered sections and the embedded object schema so a form never collects a
36+
value the submit refuses, and `GET /forms/:slug/lookup/:field` refuses a
37+
`publicPicker` declared on a managed anchor (which would have opened
38+
anonymous `sys_user` search through `owner_id`).
39+
40+
Authenticated writes are unaffected — this is the anonymous-surface rule only;
41+
`owner_id` transfer semantics for signed-in callers stay governed by the
42+
transfer grant (#3004 / PR #3018).

content/docs/ui/public-data-collection.mdx

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -45,6 +45,8 @@ POST /api/v1/forms/contact-us/submit → INSERT one showcase_inquiry
4545

4646
Only the fields listed in `sections[].fields` are accepted on submit; everything else is stripped server-side. Server-controlled fields (`status`, `source`, owner stamps) are set by a defaults hook, never by the submitter. The submit route derives a narrow `publicFormGrant: { object: 'showcase_inquiry' }` — create + read-back of the row it just made, and nothing else.
4747

48+
System-managed anchors (`owner_id`, `organization_id`, audit columns, `id`) are **never** accepted from a public submission — not even when a section declares them, and not through the zero-sections all-fields fallback. The route filter and the data layer enforce the same set (`PUBLIC_FORM_SERVER_MANAGED_FIELDS` from `@objectstack/spec/security`), so an anonymous visitor cannot forge record ownership or tenant placement regardless of how the form is declared (#3022).
49+
4850
### 3. Keep the object private once inside
4951

5052
Set `sharingModel: 'private'` on the object so submissions are staff-scoped after creation. The public path only ever **inserts**; it never lists.

packages/dogfood/test/authz-conformance.matrix.ts

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -120,6 +120,10 @@ export const AUTHZ_CONFORMANCE: AuthzPrimitive[] = [
120120
{ id: 'bulk-write-owner-scoping', summary: 'bulk (multi) update/delete are owner-scoped on OWD-private objects, not just single-id writes (#2982)', state: 'enforced',
121121
enforcement: 'objectql/engine.ts seeds opCtx.ast for no-single-id update/delete BEFORE the middleware chain and hands the composed AST to driver.updateMany/deleteMany, so plugin-sharing buildWriteFilter (owner-match + shares) and plugin-security RLS write filters actually bind bulk writes',
122122
proof: 'owner-anchor-and-bulk-writes.dogfood.test.ts' },
123+
{ id: 'public-form-managed-anchors', summary: 'anonymous public-form submit cannot supply server-managed anchors (owner_id / organization_id / audit / id) — #3022', state: 'enforced',
124+
enforcement: 'spec/security/public-form.ts PUBLIC_FORM_SERVER_MANAGED_FIELDS shared by rest/rest-server.ts form routes (allow-list + schema/section/lookup exposure) AND plugin-security publicFormGrant branch (strips every insert row before the grant admits the write — the data-layer boundary the grant otherwise bypasses; complements the #3004 step 3.5 guard, which the grant short-circuits)',
125+
proof: 'showcase-public-form.dogfood.test.ts',
126+
note: 'Proof file carries the forged owner_id/organization_id submit case; the route-level matrix is covered unit-side in rest public-form-routes.test.ts + plugin-security security-plugin.test.ts (publicFormGrant strip suite).' },
123127
{ id: 'record-share', summary: 'manual record shares (sys_record_share)', state: 'enforced',
124128
enforcement: 'plugin-sharing/sharing-service.ts buildReadFilter/canEdit' },
125129
{ id: 'sharing-rules', summary: 'criteria/owner sharing rules', state: 'enforced',

packages/dogfood/test/showcase-public-form.dogfood.test.ts

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -63,6 +63,28 @@ describe('showcase: web-to-lead public form (ADR-0056 Option A)', () => {
6363
expect(body.record.source).toBe('web');
6464
});
6565

66+
it('a forged owner_id / organization_id never lands on the row (#3022)', async () => {
67+
const r = await stack.api('/forms/contact-us/submit', {
68+
method: 'POST',
69+
headers: { 'content-type': 'application/json' },
70+
body: JSON.stringify({
71+
name: 'Mallory',
72+
email: 'mallory@example.com',
73+
message: 'Attach this record to the victim.',
74+
owner_id: 'usr_victim', // ← ownership forge (#3004-class, no credentials)
75+
organization_id: 'org_victim', // ← cross-tenant landing attempt
76+
created_by: 'usr_victim',
77+
}),
78+
});
79+
expect(r.status, 'the submit itself still succeeds — the anchors are stripped, not fatal').toBe(201);
80+
const body = (await r.json()) as { record: Record<string, unknown> };
81+
expect(body.record.name).toBe('Mallory');
82+
// The anchors are server-managed on this surface: never the forged values.
83+
expect(body.record.owner_id ?? null, 'anonymous submission must not forge ownership').not.toBe('usr_victim');
84+
expect(body.record.organization_id ?? null, 'anonymous submission must not land cross-tenant').not.toBe('org_victim');
85+
expect(body.record.created_by ?? null).not.toBe('usr_victim');
86+
});
87+
6688
it('the public grant is create + read-back ONLY — anonymous cannot list inquiries', async () => {
6789
const r = await stack.api('/data/showcase_inquiry');
6890
expect(r.status, 'general anonymous read must NOT be opened by the form grant').not.toBe(200);

packages/plugins/plugin-security/src/security-plugin.test.ts

Lines changed: 77 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -430,6 +430,83 @@ describe('SecurityPlugin', () => {
430430
expect(opCtx.ast.where).toBeUndefined();
431431
});
432432

433+
// ── publicFormGrant — server-managed anchor stripping (#3022) ───────────
434+
// The ADR-0056 Option A grant short-circuits the middleware BEFORE every
435+
// write gate (FLS 2.5, owner anchor 3.5, tenant CHECK), so the grant branch
436+
// itself must force the system-managed anchors: an anonymous public-form
437+
// insert can never forge `owner_id` / `organization_id` / audit columns,
438+
// no matter what the route-side field allow-list let through.
439+
describe('publicFormGrant — server-managed anchor stripping (#3022)', () => {
440+
const anonFormCtx = { publicFormGrant: { object: 'task' }, permissions: ['guest_portal'], anonymous: true };
441+
442+
const boot = async () => {
443+
const plugin = new SecurityPlugin({ fallbackPermissionSet: 'member_default' });
444+
const harness = makeMiddlewareCtx({ permissionSets: [tenantPolicySet] });
445+
await plugin.init(harness.ctx);
446+
await plugin.start(harness.ctx);
447+
return harness;
448+
};
449+
450+
it('strips forged owner_id / organization_id / audit anchors from an insert', async () => {
451+
const harness = await boot();
452+
const opCtx: any = {
453+
object: 'task', operation: 'insert',
454+
data: {
455+
name: 'Ada', id: 'rec_forged', owner_id: 'usr_victim', organization_id: 'org_victim',
456+
created_by: 'usr_victim', updated_by: 'usr_victim',
457+
created_at: '2020-01-01T00:00:00Z', updated_at: '2020-01-01T00:00:00Z',
458+
},
459+
context: anonFormCtx,
460+
};
461+
await harness.run(opCtx);
462+
// Business fields survive; every server-managed anchor is gone
463+
// (ownership stays unset for hooks / first-admin bootstrap to assign).
464+
expect(opCtx.data).toEqual({ name: 'Ada' });
465+
});
466+
467+
it('strips anchors from EVERY row of a batch insert', async () => {
468+
const harness = await boot();
469+
const opCtx: any = {
470+
object: 'task', operation: 'insert',
471+
data: [
472+
{ name: 'A', owner_id: 'usr_victim' },
473+
{ name: 'B', organization_id: 'org_victim' },
474+
{ name: 'C' },
475+
],
476+
context: anonFormCtx,
477+
};
478+
await harness.run(opCtx);
479+
expect(opCtx.data).toEqual([{ name: 'A' }, { name: 'B' }, { name: 'C' }]);
480+
});
481+
482+
it('a clean insert passes through untouched', async () => {
483+
const harness = await boot();
484+
const opCtx: any = {
485+
object: 'task', operation: 'insert',
486+
data: { name: 'Ada', email: 'ada@example.com' },
487+
context: anonFormCtx,
488+
};
489+
await harness.run(opCtx);
490+
expect(opCtx.data).toEqual({ name: 'Ada', email: 'ada@example.com' });
491+
});
492+
493+
it('the grant still refuses non-granted operations and foreign objects', async () => {
494+
const harness = await boot();
495+
await expect(
496+
harness.run({
497+
object: 'task', operation: 'update', data: { owner_id: 'usr_victim' },
498+
context: anonFormCtx,
499+
}),
500+
).rejects.toThrow(/public-form grant permits only create\/read-back/);
501+
await expect(
502+
harness.run({
503+
object: 'sys_user', operation: 'insert', data: { name: 'x' },
504+
context: anonFormCtx,
505+
}),
506+
).rejects.toThrow(/public-form grant permits only create\/read-back/);
507+
});
508+
});
509+
433510
// ── Row-level WRITE authorization (pre-image check, #1985) ──────────────
434511
// A by-id update/delete never builds an RLS `where`, so the owner/tenant
435512
// predicate must be enforced by re-reading the target row before mutating.

packages/plugins/plugin-security/src/security-plugin.ts

Lines changed: 34 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@
22

33
import { Plugin, PluginContext } from '@objectstack/core';
44
import type { PermissionSet, RowLevelSecurityPolicy } from '@objectstack/spec/security';
5-
import { describeHighPrivilegeBits, describeAnchorForbiddenBits } from '@objectstack/spec/security';
5+
import { describeHighPrivilegeBits, describeAnchorForbiddenBits, PUBLIC_FORM_SERVER_MANAGED_FIELDS } from '@objectstack/spec/security';
66
import { MCP_AGENT_PERMISSION_SET_RESTRICTED } from '@objectstack/spec/ai';
77
import { PermissionEvaluator, crudBucketForOperation } from './permission-evaluator.js';
88
import { DelegatedAdminGate } from './delegated-admin-gate.js';
@@ -569,7 +569,39 @@ export class SecurityPlugin implements Plugin {
569569
const allowed =
570570
opCtx.object === grantObject &&
571571
['insert', 'find', 'findOne', 'count'].includes(opCtx.operation);
572-
if (allowed) return next();
572+
if (allowed) {
573+
// [#3022] The grant bypasses every downstream write gate (FLS 2.5,
574+
// owner anchor 3.5, tenant CHECK) — so the system-managed anchors
575+
// must be forced HERE, before the write is admitted. An anonymous
576+
// submission has no principal to backfill from and no conceivable
577+
// transfer authorization, so a supplied `owner_id` /
578+
// `organization_id` / audit column is stripped from every row (the
579+
// route-side field allow-list is the first net; this is the
580+
// data-layer boundary that holds even if a FormView declares — or
581+
// a zero-section form falls open to — one of these columns).
582+
// Ownership stays NULL for object hooks / the first-admin
583+
// bootstrap to assign, exactly like other anonymous-seeded rows.
584+
if (opCtx.operation === 'insert' && opCtx.data && typeof opCtx.data === 'object') {
585+
const rows = Array.isArray(opCtx.data) ? opCtx.data : [opCtx.data];
586+
const stripped = new Set<string>();
587+
for (const row of rows) {
588+
if (!row || typeof row !== 'object' || Array.isArray(row)) continue;
589+
for (const field of PUBLIC_FORM_SERVER_MANAGED_FIELDS) {
590+
if (Object.prototype.hasOwnProperty.call(row, field)) {
591+
delete (row as Record<string, unknown>)[field];
592+
stripped.add(field);
593+
}
594+
}
595+
}
596+
if (stripped.size > 0) {
597+
ctx.logger.warn(
598+
`[security] public-form insert on '${grantObject}' supplied server-managed ` +
599+
`field(s) [${[...stripped].join(', ')}] — stripped (#3022)`,
600+
);
601+
}
602+
}
603+
return next();
604+
}
573605
throw new PermissionDeniedError(
574606
`[Security] Access denied: public-form grant permits only create/read-back on '${grantObject}', ` +
575607
`not '${opCtx.operation}' on '${opCtx.object}'`,

0 commit comments

Comments
 (0)