Skip to content

Commit 9f423dc

Browse files
os-zhuangclaude
andcommitted
fix(objectql): validate declared required organization_id/tenant_id (provenance, not name) (#1592)
`validateRecord` SKIP_FIELDS skipped required-checks for any field literally named `organization_id`/`tenant_id`. That's correct only for the engine-INJECTED tenant column (marked `system: true`, already skipped by the `def.system || def.readonly` provenance check). A genuinely DECLARED required business field with that name — `sys_team.organization_id` (a `managedBy: 'better-auth'` table where the column is NOT injected) — was silently bypassed → reached the driver as NULL (SQLITE NOT NULL constraint instead of a clean `400 { organization_id, required }`). Fix: drop `organization_id`/`tenant_id` from the by-name SKIP_FIELDS set; injected system columns stay skipped via provenance (`def.system`/`def.readonly`). Regression test (declared required org_id/tenant_id now 400s; injected system org_id still skipped). objectql suite 616 pass. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
1 parent d13df3f commit 9f423dc

3 files changed

Lines changed: 65 additions & 2 deletions

File tree

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,15 @@
1+
---
2+
"@objectstack/objectql": patch
3+
---
4+
5+
fix(objectql): validate a declared required `organization_id`/`tenant_id` instead of silently skipping it by name (#1592)
6+
7+
`validateRecord` skipped required-checks for any field literally named
8+
`organization_id` / `tenant_id`. That's correct only for the engine-INJECTED
9+
tenant column (already marked `system: true`, skipped via provenance). A
10+
genuinely DECLARED required business field with that name — e.g. `sys_team`'s
11+
`organization_id` lookup, on a `managedBy: 'better-auth'` table where the column
12+
is NOT injected — was silently bypassed and reached the driver as NULL (a DB
13+
constraint error instead of a clean `400 required`). Removed the two names from
14+
the by-name skip set; injected columns remain skipped via `def.system` /
15+
`def.readonly`.

packages/objectql/src/validation/record-validator.ts

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,8 @@
2121
* - date / datetime: must be ISO-parsable
2222
*
2323
* System-injected fields (`id`, `created_at`, `created_by`,
24-
* `updated_at`, `updated_by`, `organization_id`) are never validated
24+
* `updated_at`, `updated_by`, and provenance-flagged `system`/`readonly`
25+
* columns such as an injected `organization_id`) are never validated
2526
* here — the engine and the audit plugin manage them.
2627
*
2728
* On failure, a `ValidationError` is thrown with `.fields[]` holding
@@ -30,9 +31,16 @@
3031
* the UI can highlight the specific input.
3132
*/
3233

34+
// Lifecycle columns the engine always owns and the client never supplies. These
35+
// are skipped by NAME because they are not author-declared business fields.
36+
// NOTE: `organization_id` / `tenant_id` are intentionally NOT here (#1592) — the
37+
// engine-injected tenant column is marked `system: true` and skipped via
38+
// provenance below, while a genuinely DECLARED required `organization_id`
39+
// business field (e.g. `sys_team`, a `managedBy: 'better-auth'` table where the
40+
// column is not injected) must get a normal required-check instead of silently
41+
// passing NULL through to the driver.
3342
const SKIP_FIELDS = new Set<string>([
3443
'id', 'created_at', 'created_by', 'updated_at', 'updated_by',
35-
'organization_id', 'tenant_id',
3644
]);
3745

3846
const EMAIL_RE = /^[^\s@]+@[^\s@]+\.[^\s@]+$/;
Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,40 @@
1+
// Copyright (c) 2025 ObjectStack. Licensed under the Apache-2.0 license.
2+
3+
/**
4+
* #1592 — `validateRecord` skipped required-checks for any field literally named
5+
* `organization_id` / `tenant_id`. That's correct only for ENGINE-INJECTED tenant
6+
* columns (already marked `system: true`); a genuinely declared, required business
7+
* field with that name was silently bypassed → reached the DB as NULL. Skip by
8+
* provenance (`def.system` / `def.readonly`), never by hardcoded name.
9+
*/
10+
import { describe, it, expect } from 'vitest';
11+
import { validateRecord, ValidationError } from './record-validator';
12+
13+
describe('validateRecord provenance-aware skip (#1592)', () => {
14+
it('enforces required on a DECLARED business organization_id (not system)', () => {
15+
const schema: any = { name: 'sys_team', fields: {
16+
name: { type: 'text', required: true },
17+
organization_id: { type: 'lookup', reference: 'sys_organization', required: true },
18+
} };
19+
let caught: any;
20+
try { validateRecord(schema, { name: 'x' }, 'insert'); } catch (e) { caught = e; }
21+
expect(caught).toBeInstanceOf(ValidationError);
22+
expect(caught.fields.some((f: any) => f.field === 'organization_id' && f.code === 'required')).toBe(true);
23+
});
24+
25+
it('still skips an INJECTED system organization_id (system: true)', () => {
26+
const schema: any = { name: 'normal', fields: {
27+
name: { type: 'text', required: true },
28+
organization_id: { type: 'lookup', reference: 'sys_organization', required: false, system: true, readonly: true },
29+
} };
30+
expect(() => validateRecord(schema, { name: 'x' }, 'insert')).not.toThrow();
31+
});
32+
33+
it('enforces required on a declared tenant_id too', () => {
34+
const schema: any = { name: 't', fields: {
35+
name: { type: 'text', required: true },
36+
tenant_id: { type: 'text', required: true },
37+
} };
38+
expect(() => validateRecord(schema, { name: 'x' }, 'insert')).toThrow(ValidationError);
39+
});
40+
});

0 commit comments

Comments
 (0)