Skip to content

Commit f64668d

Browse files
os-trumpclaude
andauthored
fix(plugin-audit,plugin-security): declare sourced bounds on four keyed text columns (#12143)
Four text columns that a declared index keys on carried no `maxLength`, so driver-sql emitted them TEXT. MySQL refuses a TEXT/BLOB column in a key without a key length (ER_BLOB_KEY_WITHOUT_LENGTH): CREATE TABLE succeeds, ADD INDEX fails, and the object lands registered-but-broken with its declared index silently absent. Each bound is derived from a named producer, stated in the declaration so it is vetoable in review (route A): sys_activity.record_id 255 physical `id` column sys_audit_log.record_id 255 physical `id` column sys_audience_binding_suggestion.package_id 255 sys_permission_set.package_id sys_audience_binding_suggestion.permission_set_name 100 sys_permission_set.name None narrows anything storable: a record id cannot exceed the varchar(255) the id itself lives in, and a permission set name over 100 is already refused at the write seam today (measured: "API Name must be <= 100 characters (got 101)"). Each plugin also gains a keyed-text-bounds pin driven through its own registration path rather than a hand-written object list — the platform-objects pin enumerates only that package's exports, which is why these columns escaped route A's sweep after ADR-0029 K2 moved the objects out. Claude-Session: https://claude.ai/code/session_01UQgPSniH1GFM9ZDeGyuGUa Co-authored-by: Claude <noreply@anthropic.com>
1 parent eeec62a commit f64668d

6 files changed

Lines changed: 494 additions & 0 deletions

File tree

Lines changed: 48 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,48 @@
1+
---
2+
"@objectstack/plugin-audit": patch
3+
"@objectstack/plugin-security": patch
4+
---
5+
6+
fix(plugin-audit,plugin-security): declare sourced bounds on the four keyed text columns that break MySQL schema-sync (#12059)
7+
8+
Four text columns that a declared index keys on carried no `maxLength`, so
9+
`driver-sql` emitted them `TEXT`. MySQL refuses a TEXT/BLOB column in a key
10+
without a key length (`ER_BLOB_KEY_WITHOUT_LENGTH`): `CREATE TABLE` succeeds,
11+
`ALTER TABLE … ADD INDEX` fails, and the object lands registered-but-broken
12+
with its declared index silently absent.
13+
14+
| Object | Column | Bound | Producer the bound is derived from |
15+
|---|---|---|---|
16+
| `sys_activity` | `record_id` | 255 | the physical `id` column — `driver-sql` creates every primary key as `table.string('id').primary()`, knex's `varchar(255)` |
17+
| `sys_audit_log` | `record_id` | 255 | same |
18+
| `sys_audience_binding_suggestion` | `package_id` | 255 | `sys_permission_set.package_id` (255), which the same boot pass writes the same value into |
19+
| `sys_audience_binding_suggestion` | `permission_set_name` | 100 | `sys_permission_set.name` (100), the column this value resolves against at confirm time |
20+
21+
Each bound is derived from a **named producer** and stated in the declaration
22+
so it is vetoable in review (#11374 route A; PR #12058 is the worked
23+
precedent). None of them narrows anything storable:
24+
25+
- a record id cannot exceed the `varchar(255)` column the id itself lives in,
26+
and the `referenceVia` seed path refuses an unresolvable pointer rather than
27+
storing a natural key verbatim;
28+
- a permission set name longer than 100 is already refused at the write seam
29+
today — measured on a real engine, `ValidationError: API Name must be ≤ 100
30+
characters (got 101)` — so no set with such a name can exist, and a
31+
suggestion naming one could never be confirmed.
32+
33+
Measured at the driver level, shipped declaration vs. the same declaration with
34+
the bounds stripped: `record_id`, `package_id` and `permission_set_name` move
35+
`TEXT``varchar(255)` / `varchar(100)`, while `id` reads `varchar(255)` in
36+
both — the transitivity premise, read off a real table rather than assumed.
37+
38+
Existing deployments are not rewritten: a physical `TEXT` column is deliberately
39+
not diffed against `maxLength` (#11431), so no `ALTER` is planned and no value
40+
at rest is truncated. The repair takes effect where the decision is makeable at
41+
all — at `CREATE TABLE` — because no dialect turns a TEXT column into a keyable
42+
one afterwards.
43+
44+
Each plugin also gains a keyed-text-bounds pin driven through its **own
45+
registration path** (`init()` → the manifest `register({ objects })` call),
46+
rather than a hand-written object list: the platform-objects pin enumerates only
47+
that package's exports, which is exactly why these four columns escaped route
48+
A's sweep after ADR-0029 K2 moved the objects out.

packages/plugins/plugin-audit/src/objects/sys-activity.object.ts

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -173,11 +173,37 @@ export const SysActivity = ObjectSchema.create({
173173
group: 'Target',
174174
}),
175175

176+
// [#11374 route A] The value is a record id of the object `object_name`
177+
// names — written by `audit-writers.ts` (`record_id: recordId`, the id of
178+
// the very row the mutation touched). The bound is derived by
179+
// referenced-column transitivity from the id itself, never guessed:
180+
// `driver-sql` creates every table's primary key as
181+
// `table.string('id').primary()` — knex's `varchar(255)`, which the driver
182+
// spells out as `DEFAULT_STRING_VARCHAR_CHARS = 255` and names in its own
183+
// error text as "built-in `id` (a varchar(255))". No id this column can
184+
// receive is wider than the column the id lives in.
185+
//
186+
// The seed path cannot widen it either: an unresolvable pointer is
187+
// "refused loudly, never stored verbatim" (`metadata-protocol`'s
188+
// seed-loader), so `referenceVia` resolves a natural key to a real record
189+
// id BEFORE it is stored — a raw external key never lands in this column.
190+
//
191+
// 255 rather than the 100 that `plugin-sharing` and `plugin-approvals`
192+
// chose for their own `record_id`: those narrow below what the id column
193+
// itself accepts, which is safe only for their own writers. 255 is the
194+
// transitive ceiling, so it refuses nothing that is storable today.
195+
// It is also <= the 768-character utf8mb4 key ceiling, so the
196+
// `(object_name, record_id)` index below is expressible on MySQL — which is
197+
// the whole point: unbounded, this column was emitted TEXT, MySQL refused
198+
// the index with `ER_BLOB_KEY_WITHOUT_LENGTH`, and the object landed
199+
// registered-but-broken with the ActivityPointer lookup the read path
200+
// assumes (ADR-0052 §5) silently absent.
176201
record_id: Field.text({
177202
label: 'Record ID',
178203
required: false,
179204
readonly: true,
180205
searchable: true,
206+
maxLength: 255,
181207
// [#11339] The id half of the ActivityPointer pair (ADR-0052 §5): a
182208
// record id of the object `object_name` names on the same row. Declaring
183209
// it makes the pair seedable — a packaged app's seed writes the target's

packages/plugins/plugin-audit/src/objects/sys-audit-log.object.ts

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -226,11 +226,25 @@ export const SysAuditLog = ObjectSchema.create({
226226
group: 'Target',
227227
}),
228228

229+
// [#11374 route A] The bound is derived by referenced-column transitivity
230+
// from the id this column holds, never guessed: `driver-sql` creates every
231+
// table's primary key as `table.string('id').primary()` — knex's
232+
// `varchar(255)`, which the driver spells out as
233+
// `DEFAULT_STRING_VARCHAR_CHARS = 255` and names in its own error text as
234+
// "built-in `id` (a varchar(255))". The writers enumerated below all stamp
235+
// a real record id of a stored row, so none of them can produce a value
236+
// wider than the column that id lives in. 255 is also <= the 768-character
237+
// utf8mb4 key ceiling, so the `(object_name, record_id)` index below is
238+
// expressible on MySQL — which is the whole point: unbounded, this column
239+
// was emitted TEXT, MySQL refused the index with
240+
// `ER_BLOB_KEY_WITHOUT_LENGTH`, and the object landed registered-but-broken
241+
// with the lookup its own `record_views` list view depends on absent.
229242
record_id: Field.text({
230243
label: 'Record ID',
231244
required: false,
232245
readonly: true,
233246
searchable: true,
247+
maxLength: 255,
234248
description: 'ID of the affected record',
235249
// [#11386] The id half of this object's ActivityPointer pair (ADR-0052
236250
// §5), adopting the #11339 carrier. VERIFIED for THIS object rather than
Lines changed: 178 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,178 @@
1+
// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license.
2+
3+
import { describe, it, expect } from 'vitest';
4+
import { AuditPlugin } from './audit-plugin.js';
5+
6+
/**
7+
* #11374 route A, for the objects THIS PLUGIN registers — every text-family
8+
* column a declared index keys on must declare a `maxLength`, because a bound
9+
* is what lets the column be a key at all.
10+
*
11+
* ## Why a second copy of the pin lives here
12+
*
13+
* The original pin is `@objectstack/platform-objects`'
14+
* `platform-keyed-text-bounds.test.ts`, and it enumerates the objects THAT
15+
* package exports. Platform objects that moved out to plugins under ADR-0029 K2
16+
* are outside it by construction, which is exactly how `sys_activity.record_id`
17+
* and `sys_audit_log.record_id` stayed unbounded through route A's sweep: the
18+
* pin could not see them, so nothing failed by name.
19+
*
20+
* That is the same failure the platform pin already survived once at a smaller
21+
* scale (it used to be scoped to `identity/`, and `sys_import_job.created_by`
22+
* in `audit/` escaped it). A pin scoped to a package polices a package, not the
23+
* defect class. The class-level repair — one walk over every package that ships
24+
* platform objects — is engine-lane work tracked separately; until it lands,
25+
* each shipping package carries its own copy so no keyed column is unpoliced.
26+
*
27+
* ## Why it drives `init()` instead of importing the objects
28+
*
29+
* This package's `package.json` declares only the `.` export and the root
30+
* barrel does not re-export `./objects`, so nothing outside the package can
31+
* import `SysActivity` at all — which is why the objects were never measured
32+
* live. Enumerating a hand-written list here would reproduce that blind spot in
33+
* miniature: the list, not the plugin, would define the surface. So the pin
34+
* drives the REAL registration path (`AuditPlugin.init` → the `manifest`
35+
* service's `register({ objects })`) and polices whatever the plugin actually
36+
* contributes to a kernel. An object added to that call is policed the moment
37+
* it is added, with no second edit here.
38+
*
39+
* ## What a red on this file means
40+
*
41+
* A new keyed text-family field arrived without a `maxLength`. Do not silence
42+
* it — derive a bound from the value's producer and declare it (route A's
43+
* shape: a NAMED producer, stated in the declaration so it is vetoable in
44+
* review), or extend the allowlist with a comment naming why no bound exists
45+
* and where the keyability debt is tracked.
46+
*
47+
* On MySQL the cost of a red is not theoretical: the unbounded column is
48+
* emitted `TEXT`, `ALTER TABLE … ADD INDEX` is refused with
49+
* `ER_BLOB_KEY_WITHOUT_LENGTH`, and the object lands registered-but-broken with
50+
* its declared index silently absent.
51+
*/
52+
53+
const TEXT_FAMILY = new Set(['text', 'textarea', 'html', 'markdown']);
54+
55+
/**
56+
* Keyed text-family columns with NO defensible bound. Every entry must name
57+
* why. Entries that stop matching a real keyed unbounded column fail the last
58+
* test, so the list cannot rot. Empty today, deliberately: all four of this
59+
* plugin's keyed text columns have a sourced bound.
60+
*/
61+
const UNBOUNDABLE: ReadonlySet<string> = new Set([]);
62+
63+
type AnyObject = {
64+
name: string;
65+
fields: Record<string, { type?: string; maxLength?: unknown }>;
66+
indexes?: Array<{ fields?: string[]; unique?: boolean | string }>;
67+
};
68+
69+
/**
70+
* The objects `AuditPlugin` really contributes to a kernel, read off the
71+
* manifest registration it performs in `init()`.
72+
*/
73+
async function registeredObjects(): Promise<AnyObject[]> {
74+
const captured: AnyObject[] = [];
75+
const noop = () => {};
76+
const logger = {
77+
info: noop, warn: noop, error: noop, debug: noop,
78+
child() { return logger; },
79+
};
80+
const ctx = {
81+
logger,
82+
getService(name: string) {
83+
if (name === 'manifest') {
84+
return {
85+
register(m: { objects?: AnyObject[] }) {
86+
for (const o of m?.objects ?? []) captured.push(o);
87+
},
88+
};
89+
}
90+
return undefined;
91+
},
92+
registerService: noop,
93+
hook: noop,
94+
} as never;
95+
96+
await new AuditPlugin().init(ctx);
97+
return captured;
98+
}
99+
100+
function keyedTextColumns(o: AnyObject): Array<{ column: string; maxLength: unknown }> {
101+
const keyed = new Set<string>();
102+
for (const ix of o.indexes ?? []) for (const f of ix.fields ?? []) keyed.add(f);
103+
return Object.entries(o.fields ?? {})
104+
.filter(([name, def]) => keyed.has(name) && TEXT_FAMILY.has(def?.type ?? ''))
105+
.map(([column, def]) => ({ column: `${o.name}.${column}`, maxLength: def.maxLength }));
106+
}
107+
108+
describe('plugin-audit keyed text-family columns declare their bound (#11374 route A)', () => {
109+
it('enumerates a real surface through the plugin registration path — the probe is not vacuous', async () => {
110+
// Positive control: if `init()` stops registering objects, or the field /
111+
// index spelling changes so this file stops seeing columns, fail loudly
112+
// instead of passing empty. An empty enumeration is the failure mode that
113+
// let these columns escape route A in the first place.
114+
const objects = await registeredObjects();
115+
expect(objects.map((o) => o.name)).toEqual(
116+
expect.arrayContaining(['sys_audit_log', 'sys_activity', 'sys_comment']),
117+
);
118+
119+
// 5 is MEASURED off this registration surface, not a round number:
120+
// sys_audit_log.{object_name,record_id}, sys_activity.{object_name,record_id},
121+
// sys_comment.thread_id. Every other index on these three objects keys on a
122+
// lookup, select or datetime column, which is not text-family.
123+
const all = objects.flatMap(keyedTextColumns);
124+
expect(all.length).toBeGreaterThanOrEqual(5);
125+
// Three names from THREE DIFFERENT objects, so a future narrowing of the
126+
// enumeration fails here by name rather than by quietly enumerating less.
127+
expect(all.map((c) => c.column)).toContain('sys_activity.record_id');
128+
expect(all.map((c) => c.column)).toContain('sys_audit_log.record_id');
129+
expect(all.map((c) => c.column)).toContain('sys_comment.thread_id');
130+
});
131+
132+
it('every keyed text-family column declares a positive integer maxLength, or is allowlisted by name', async () => {
133+
const objects = await registeredObjects();
134+
const offenders: string[] = [];
135+
for (const o of objects) {
136+
for (const { column, maxLength } of keyedTextColumns(o)) {
137+
if (UNBOUNDABLE.has(column)) continue;
138+
const bounded =
139+
typeof maxLength === 'number' && Number.isInteger(maxLength) && maxLength > 0;
140+
if (!bounded) offenders.push(`${column} (maxLength: ${String(maxLength)})`);
141+
}
142+
}
143+
expect(
144+
offenders,
145+
`keyed text-family column(s) without a declared maxLength — on MySQL their ` +
146+
`declared index cannot be created and the object lands registered-but-broken. ` +
147+
`Declare a sourced bound or extend UNBOUNDABLE with a named reason: ` +
148+
offenders.join(', '),
149+
).toEqual([]);
150+
});
151+
152+
it('the two ActivityPointer id columns carry the referenced-column bound, not just any bound', async () => {
153+
// The bound is not free-floating: 255 is the width of the physical `id`
154+
// column `driver-sql` creates (`table.string('id').primary()`, knex's
155+
// varchar(255) — the driver spells it `DEFAULT_STRING_VARCHAR_CHARS`), so a
156+
// column holding a record id is bounded by transitivity from the id itself.
157+
// Pinned by VALUE because a later edit that "tidies" one of these to a
158+
// narrower sibling convention (100, as plugin-sharing and plugin-approvals
159+
// chose) would silently make the column unable to hold ids that the id
160+
// column itself accepts.
161+
const byName = new Map((await registeredObjects()).map((o) => [o.name, o]));
162+
expect(byName.get('sys_activity')?.fields.record_id?.maxLength).toBe(255);
163+
expect(byName.get('sys_audit_log')?.fields.record_id?.maxLength).toBe(255);
164+
});
165+
166+
it('the UNBOUNDABLE allowlist matches only real, still-unbounded keyed columns', async () => {
167+
const real = new Map(
168+
(await registeredObjects()).flatMap(keyedTextColumns).map((c) => [c.column, c.maxLength]),
169+
);
170+
for (const entry of UNBOUNDABLE) {
171+
expect(real.has(entry), `allowlist entry ${entry} is not a keyed text column any more — remove it`).toBe(true);
172+
expect(
173+
real.get(entry),
174+
`allowlist entry ${entry} now declares a bound — remove it from UNBOUNDABLE`,
175+
).toBeUndefined();
176+
}
177+
});
178+
});

packages/plugins/plugin-security/src/objects/sys-audience-binding-suggestion.object.ts

Lines changed: 62 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -41,17 +41,79 @@ export const SysAudienceBindingSuggestion = ObjectSchema.create({
4141
description: 'UUID of the suggestion row.',
4242
}),
4343

44+
// [#11374 route A] Both key columns below declare a bound derived by
45+
// referenced-column transitivity, and the producer is named per column so
46+
// the derivation is vetoable in review rather than taken on trust. The pair
47+
// is the object's declared unique key `(package_id, permission_set_name,
48+
// anchor)`; unbounded, both were emitted TEXT, MySQL refused the index with
49+
// `ER_BLOB_KEY_WITHOUT_LENGTH`, and the object landed registered-but-broken
50+
// — the per-tenant uniqueness this table depends on silently absent.
51+
//
52+
// The transitivity is not an analogy here, it is the confirm path itself:
53+
// `confirmAudienceBindingSuggestion` resolves the row by
54+
// `find('sys_permission_set', { name: row.permission_set_name })` and, when
55+
// that misses, materializes it via `upsertPackagePermissionSet(ql,
56+
// declared.set, row.package_id)` — which writes these two values into
57+
// `sys_permission_set.name` and `sys_permission_set.package_id`. So a
58+
// suggestion is confirmable exactly when its two key values fit the columns
59+
// `sys_permission_set` declares, and those columns are what bound these.
60+
4461
package_id: Field.text({
4562
label: 'Package',
4663
required: true,
4764
readonly: true,
65+
// Producer: the owning package's manifest id (`manifest.id`, or the
66+
// `_packageId` the metadata layer stamps) — `collectDeclaredSuggestions`
67+
// reads one of those two and `syncAudienceBindingSuggestions` writes it
68+
// here verbatim. Bounded at 255 because the SAME boot pass writes the
69+
// SAME value into `sys_permission_set.package_id` (maxLength: 255), and
70+
// every landed column of this value class agrees: `sys_capability
71+
// .package_id`, `sys_metadata.package_id`, `sys_metadata_commit
72+
// .package_id`. A package id too wide for those cannot own a
73+
// materialized permission set, so it cannot produce a confirmable
74+
// suggestion either. Measured against the in-repo corpus, the longest
75+
// real reverse-domain package id is 57 characters — the floor is cleared
76+
// with room to spare.
77+
maxLength: 255,
4878
description: 'Owning package that ships the suggested permission set (ADR-0086 D3 provenance).',
4979
}),
5080

5181
permission_set_name: Field.text({
5282
label: 'Permission Set',
5383
required: true,
5484
readonly: true,
85+
// Producer: the declared set's own `name` (spec `PermissionSetSchema
86+
// .name`), written here as `d.set.name`. Bounded at 100 by
87+
// referenced-column transitivity from `sys_permission_set.name`
88+
// (maxLength: 100) — the column this value must resolve against, as this
89+
// field's own description says and as the confirm path literally does.
90+
//
91+
// The ceiling is MEASURED at the write seam, not inferred from the
92+
// declaration. On a real ObjectQL engine over a real SqlDriver, inserting
93+
// a longer name into `sys_permission_set` is refused before the driver is
94+
// reached:
95+
// len 101 → ValidationError: API Name must be ≤ 100 characters (got 101)
96+
// len 120 / 255 / 256 / 300 → the same refusal
97+
// (`objectql`'s `record-validator.ts` `max_length` check). So no
98+
// permission set whose name exceeds 100 characters can exist, and a
99+
// suggestion naming one could never be confirmed:
100+
// `confirmAudienceBindingSuggestion` answers `SuggestionStateError`
101+
// ("Permission set '…' is not materialized in sys_permission_set").
102+
// Bounding at 100 therefore refuses nothing that is storable today.
103+
//
104+
// 100 and not the 255 its `package_id` sibling takes: the two columns
105+
// reference DIFFERENT columns and inherit their widths independently.
106+
// The in-package precedent for exactly this shape is
107+
// `sys_user_position.position` — "Position machine name (references
108+
// sys_position.name)", maxLength 100 against `sys_position.name`'s 100.
109+
//
110+
// ⚠️ The bound is transitive, not intrinsic: `PermissionSetSchema.name`
111+
// is `SnakeCaseIdentifierSchema`, which carries `.min(2)` and NO `.max()`,
112+
// so the SPEC does not bound identifier length — every cap on this value
113+
// class comes from the columns that store it. Filed separately; if
114+
// `sys_permission_set.name` is ever widened, the pin beside this file is
115+
// where this bound is re-derived rather than rediscovered on MySQL.
116+
maxLength: 100,
55117
description: 'Name of the suggested permission set (resolved against sys_permission_set at confirm time).',
56118
}),
57119

0 commit comments

Comments
 (0)