Skip to content

fix(plugin-audit,plugin-security): declare sourced bounds on the four keyed text columns that break MySQL schema-sync - #12143

Merged
os-trump merged 1 commit into
mainfrom
claude/issue-12059-plugin-keyed-text-bounds
Aug 25, 2026
Merged

fix(plugin-audit,plugin-security): declare sourced bounds on the four keyed text columns that break MySQL schema-sync#12143
os-trump merged 1 commit into
mainfrom
claude/issue-12059-plugin-keyed-text-bounds

Conversation

@os-trump

@os-trumpos-trump commented Aug 25, 2026

Copy link
Copy Markdown
Collaborator

Fixes#12059

Closing form confirmed by the PM (was Part of, flipped per the dev's own condition): the class-level walk — the half of #12059 its body called "the one worth fixing" — is now tracked as #12147 (source-scanning gate; pin-lift blocked by dependency direction, feasibility measurement below). With that half carried by its own card, merging this rightly closes#12059. Ruling recorded in the card's ACCEPT comment.

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,
ALTER TABLE … ADD INDEX fails, and the object lands registered-but-broken with
its declared index silently absent.

The four bounds, and the producer each is derived from

Route A's shape: every bound derived from a named producer and stated in the
declaration so it is vetoable in review (PR #12058 is the worked precedent).

ObjectColumnBoundProducer
sys_activityrecord_id255the physical id column — driver-sql creates every primary key as table.string('id').primary(), knex's varchar(255), spelled DEFAULT_STRING_VARCHAR_CHARS = 255
sys_audit_logrecord_id255same
sys_audience_binding_suggestionpackage_id255sys_permission_set.package_id (255) — the same boot pass writes the same value into it; sys_capability, sys_metadata and sys_metadata_commit agree at 255
sys_audience_binding_suggestionpermission_set_name100sys_permission_set.name (100) — the column this value resolves against at confirm time

The transitivity for the suggestion pair is not an analogy — it is the confirm
path itself. confirmAudienceBindingSuggestion resolves the row by
find('sys_permission_set', { name: row.permission_set_name }) and, on a miss,
materializes it via upsertPackagePermissionSet(ql, declared.set, row.package_id),
which writes these two values into sys_permission_set.name and
sys_permission_set.package_id. A suggestion is confirmable exactly when its two
key values fit the columns sys_permission_set declares.

Clause-② reverse-verification: no legitimately storable value is refused

The fork condition asked, per column, whether a longer value can legitimately
reach it today. Answered by measurement rather than by reading the schema:

  • record_id (both objects) — the writers (audit-writers.ts
    record_id: recordId, read-audit.ts, auth-event-audit.ts, plugin-auth's
    admin endpoints) all stamp a real record id of a stored row, and the physical
    column that id lives in is varchar(255). The seed path cannot widen it
    either: referenceVia resolution "refuses loudly, never stores verbatim" an
    unresolvable pointer, so a raw natural key never lands in the column.

  • package_id — the same value is written by the same pass into
    sys_permission_set.package_id (255). Longest real reverse-domain package id
    measured in this repo: 57 characters.

  • permission_set_name — a name over 100 is already refused at the write
    seam today
    , independent of this PR. Measured on a real ObjectQL engine over
    a real SqlDriver:

    len 101 → ValidationError: API Name must be ≤ 100 characters (got 101)
    at validateRecord (packages/objectql/src/validation/record-validator.ts:1058)
    len 120 / 255 / 256 / 300 → the same refusal
    

    So no permission set with such a name can exist, and a suggestion naming one
    could never be confirmed (SuggestionStateError: "Permission set '…' is not
    materialized in sys_permission_set"). Bounding at 100 refuses nothing storable.

    ⚠️ Worth recording explicitly: the bound is transitive, not intrinsic.
    PermissionSetSchema.name is SnakeCaseIdentifierSchema, which carries
    .min(2) and no .max() — the spec does not bound identifier length at
    all, and every cap on this value class comes from the columns that store it.
    Filed as The shared identifier schemas declare no maximum length, so every cap on an identifier is a storage accident rather than a contract #12144.

Existing deployments are not rewritten. A physical TEXT column is
deliberately not diffed against maxLength (#11431), so no ALTER is planned
and no value at rest is truncated. The repair takes effect where the decision is
makeable at all — at CREATE TABLE — because no dialect turns a TEXT column into
a keyable one afterwards.

Verification record

All results below were produced through the shared verify lock; every exit code
was captured before any pipe, and each gate's own verdict line is quoted.
Final commit 24a331cc9; git status --porcelain was empty at commit time, so
the gate-tested working tree is that commit byte-for-byte.

Ablation — direction predicted in writing before running: RED (a non-empty
offenders list), not "more diagnostics" and not a reversal.

Both new pins were run against the unfixed tree first. plugin-audit, pre-fix:

AssertionError: keyed text-family column(s) without a declared maxLength …
+ [ "sys_audit_log.record_id (maxLength: undefined)",
+ "sys_activity.record_id (maxLength: undefined)" ]
Tests 3 failed | 1 passed (4)

plugin-security, pre-fix:

+ [ "sys_audience_binding_suggestion.package_id (maxLength: undefined)",
+ "sys_audience_binding_suggestion.permission_set_name (maxLength: undefined)" ]
Tests 2 failed | 3 passed (5)

After the fix: plugin-audit 4/4 passed, plugin-security 5/5 passed.

One prediction was wrong and was corrected against the measurement rather than
the other way round
: I guessed the audit vacuity floor at ≥6 keyed text
columns; the real surface is 5 (sys_audit_log.{object_name,record_id},
sys_activity.{object_name,record_id}, sys_comment.thread_id). The floor in
the pin is now the measured 5, with a comment saying so.

Mutation confirmed on disk before any conclusion was drawn: git diff -U0
showed exactly four added maxLength: lines and zero deletions
(102 insertions total). No dist/ ablation leg applies — both pins resolve the
plugin from source inside their own package.

The card's count is confirmed.#12059 recorded its "4 unbounded keyed text
columns" as unverified, because the plugins' export surface cannot reach the
objects. Driving each plugin's real registration path enumerates the full
surface, and the offenders are exactly those four — no more, no fewer.

Driver-level type mapping (no live MySQL in this container — see NOT
MEASURED below). Shipped declaration vs. the same declaration with the bounds
stripped, read back off a real table:

### sys_audience_binding_suggestion — SHIPPED ### PRE-FIX
id -> varchar(255) id -> varchar(255)
package_id -> varchar(255) package_id -> TEXT
permission_set_name -> varchar(100) permission_set_name -> TEXT
anchor -> varchar(255) anchor -> varchar(255)
### sys_activity / sys_audit_log — SHIPPED ### PRE-FIX
id -> varchar(255) id -> varchar(255)
object_name -> varchar(255) object_name -> varchar(255)
record_id -> varchar(255) record_id -> TEXT

id -> varchar(255) in every table is the transitivity premise for the two
record_id bounds, read off a real table rather than assumed. The composite key
arithmetic also checks out against the utf8mb4 index-key ceiling:
255 + 100 + 255 = 610 ≤ 768 characters.

Suites and gates (each line is the tool's own verdict):

plugin-audit test Test Files 22 passed (22) Tests 325 passed (325)
plugin-security test Test Files 82 passed (82) Tests 1532 passed (1532)
plugin-audit typecheck: Done plugin-security typecheck: Done

Gate union re-derived on the final commit with
node scripts/pm/dispatch-gates.mjs --repo objectstack-ai/objectstack
(16 path-matched families, committed 6, working tree 0, untracked 0); all run,
all exit 0:

check:changeset-gate-self-tests · check:cross-package-test-inputs ·
check:objectui-changeset · check:published-files · check:slot-lookup ·
check:test-source-alias · check:type-source-resolution ·
check-adr-0087-registration · check-changeset-no-major ·
check-ci-filter-parity · check-empty-changeset · check-plugin-teardown-shape ·
release-rehearsal-clone --self-test · plus the convention-triggered
check:query-options-erasure · check:engine-double-contract ·
check:where-matcher · check:i18n-stale-fill · check:type-check-coverage ·
check:nul-bytes.

Two gates deserve their own line because their first result was not a pass:

  • check:i18n first exited 1 with "PREREQUISITE NOT MET — the workspace CLI is
    not built … Nothing was checked"
    . That is NOT MEASURED, not a red. After
    building the CLI it reports:
    check-i18n-bundles: OK (9 package(s) — all bundles in sync, no undeclared authoring keys),
    with plugins/plugin-audit and plugins/plugin-security each named in sync (4 bundle(s)).

  • check:type-check-debt (--re-measure) first refused: one dependency
    (@objectstack/service-knowledge) had no built type entry point, and the gate
    states plainly that measuring from there would measure a different world.
    After building exactly that closure:
    check-type-check-coverage --re-measure: OK — 32 ledger entr(ies) re-measured in 296.1s, 1897 raw tsc error(s) total, none above its recorded number.
    This matters here because @objectstack/plugin-security carries a TEST_DEBT
    entry (its tests sit outside the tsc program its own typecheck runs), so a
    new test file in that package is exactly the shape that could move the ratchet.
    It did not.

    The run also reports a pre-existing -1 surplus on @objectstack/plugin-approvals
    (records 348, measures 347). Untouched by this PR and left alone deliberately —
    out of scope: platform-objects' keyed-text-bounds pin cannot reach plugin/service packages — four keyed unbounded text columns in plugin-audit and plugin-security still break MySQL schema-sync #12059 is not addressed by lowering another package's ledger.

NOT MEASURED

The live MySQL leg. No MySQL is reachable from this container (no Docker
daemon; OS_TEST_MYSQL_URL unset), so ER_BLOB_KEY_WITHOUT_LENGTH was not
reproduced before the fix nor its absence confirmed after. The type-mapping half
above is measured; the index-refusal half is the mechanism #12058 measured live
on MySQL 8.0.46 on this exact code path. Note the SQLite readings above create
the index in both columns — SQLite keys a TEXT column happily, which is
precisely why this defect is MySQL-only and invisible on SQLite and Postgres.

Pin-lift feasibility (reported, deliberately NOT implemented — now tracked as #12147)

The class-level fix — making the platform-objects keyed-text-bounds pin walk
every package that ships platform objects — is engine-lane surface and is not in
this PR. What this card measured about its feasibility:

  1. A central pin cannot import plugin objects. Each plugin's package.json
    declares only the . export and the root barrel does not re-export
    ./objects. This is not a formality: it is why the card could not verify its
    own counts.
  2. Fixing that by importing would invert the dependency graph.
    platform-objects depends on @objectstack/metadata-core and
    @objectstack/spec only; plugin-audit and plugin-security both depend on
    platform-objects. A pin there that imported the plugins would create a
    cycle, so "lift the existing pin" is not available as written.
  3. The viable shape is a source-scanning gate, not a pin — walk every
    *.object.ts in the repo, intersect each object's indexes with its
    text-family fields, fail on an unbounded keyed column. That is the idiom the
    repo already blesses for exactly this reason ("a detector with no dependencies
    cannot itself fail to resolve in CI", per check:cross-package-test-inputs),
    it has no dependency-direction problem, and it is how platform-objects' keyed-text-bounds pin cannot reach plugin/service packages — four keyed unbounded text columns in plugin-audit and plugin-security still break MySQL schema-sync #12059's own evidence
    was produced. Its known cost is the documented weakness of source scans — it
    sees only the spellings it knows — so it would need the vacuity control the
    existing pins already carry.

Recommendation: a scripts/check-keyed-text-bounds.mjs class-level walk with a
per-package allowlist, superseding the three per-package pins. Until it exists,
each shipping package carries its own copy — which is what this PR adds for the
two plugins, driven through each plugin's own registration path
(init()manifest.register({ objects })) rather than a hand-written object
list, so an object added to a plugin is policed the moment it is added.

Scope

Three object declaration files plus one pin beside each plugin, and one
changeset covering both published plugins (patch). bootstrap-platform-admin.ts
(#11959's surface) is untouched.


Generated by Claude Code

…ed text columns
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.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01UQgPSniH1GFM9ZDeGyuGUa
@github-actions

Copy link
Copy Markdown
Contributor

📓 Docs Drift Check

This PR changes 2 package(s): @objectstack/plugin-audit, @objectstack/plugin-security, touching 4 documentable anchor(s).

4 hand-written doc(s) NAME something this change touched and may need an implementation-accuracy re-verification:

  • content/docs/api/error-catalog.mdx(via sys_permission_set (literal))
  • content/docs/permissions/authorization.mdx(via sys_permission_set (literal))
  • content/docs/permissions/delegated-administration.mdx(via sys_permission_set (literal))
  • content/docs/permissions/permission-sets.mdx(via sys_permission_set (literal))

4 release-owned page(s) also name something this change touched. These are read-only:

  • content/docs/releases/v12.mdx(via sys_permission_set (literal))
  • content/docs/releases/v13.mdx(via sys_permission_set (literal))
  • content/docs/releases/v15.mdx(via sys_permission_set (literal))
  • content/docs/releases/v17.mdx(via sys_permission_set (literal))

content/docs/releases/ is RELEASE-OWNED (AGENTS.md "Documentation Guardrails"): release
notes are written centrally at release time, and a code PR that edits them is the exact PR
that guardrail exists to stop. They are still audited — read-only. If one of them is actually
wrong, file an issue or open a dedicated docs-only PR; do not edit it here.

What this run could not see
  • a page that states a rule by its inputs shares no identifier with the emitter that implements the rule, so an emitter-only diff cannot list it — not on this run and not on any run. Measured on fix(driver-sql): emit varchar(maxLength) for a text field a declared index keys on #11430: content/docs/protocol/objectql/types.mdx documents the text-family column mapping by the ObjectQL type names it maps FROM (text / textarea / html) while the diff changed createColumn; it went unlisted, and it was the page that diff falsified, in four places. No shared token exists to detect this on, so a rule your change carries has to be re-read by hand in the pages that restate it.

Coarse fallback — 18 page(s) merely mention a changed package (the pre-#9192 predicate, kept for the deliberately-wide backstop): node scripts/docs-audit/affected-docs.mjs --json 487e3614341379860d66554b8037840c093cb590packageMentionDocs.

Which tree this was computed on

This run read content/docs from c4c388bebc57a878111c6a7b1378544a4840b487 — the merge of head 24a331cc9e42d9c6a1d7ad6d62b70294ceec22de into base 487e3614341379860d66554b8037840c093cb590, which is what actions/checkout gives a pull_request run. Not the PR head.

A worktree cut from an older main holds a different content/docs, so re-deriving there can legitimately return a different list — that is a different tree, not a wrong row. To answer on the same tree:

# while this PR is open — GitHub drops the merge commit once it closes
git fetch origin c4c388bebc57a878111c6a7b1378544a4840b487 && git checkout c4c388bebc57a878111c6a7b1378544a4840b487
# afterwards, rebuild it from the two parents, which stay fetchable
git fetch origin 487e3614341379860d66554b8037840c093cb590 24a331cc9e42d9c6a1d7ad6d62b70294ceec22de && git checkout -B drift-repro 487e3614341379860d66554b8037840c093cb590 && git merge --no-ff 24a331cc9e42d9c6a1d7ad6d62b70294ceec22de
node scripts/docs-audit/affected-docs.mjs --json 487e3614341379860d66554b8037840c093cb590

⚠️ That checkout carried uncommitted changes, so the commit above does not fully identify what was read.

Advisory only, and a precision-first one (#9192): a page is listed because it names a
symbol, wire route or SDK method this diff touched — not because it mentions a changed
package. Each row says which anchor put it there, so a wrong row is reportable rather than
merely annoying. To re-verify, run the docs-accuracy-audit workflow scoped to these files:
node scripts/docs-audit/affected-docs.mjs 487e3614341379860d66554b8037840c093cb590 → pass the list as
args.docs, on the commit named under Which tree this was computed on.

@github-actionsgithub-actionsBot added documentation Improvements or additions to documentation tests tooling labels Aug 25, 2026
@os-trump
os-trump marked this pull request as ready for review August 25, 2026 11:27
@os-trump
os-trump added this pull request to the merge queueAug 25, 2026
@github-merge-queue
github-merge-queueBot removed this pull request from the merge queue due to failed status checks Aug 25, 2026
@os-trump
os-trump added this pull request to the merge queueAug 25, 2026
Merged via the queue into main with commit f64668dAug 25, 2026
37 checks passed
@os-trump
os-trump deleted the claude/issue-12059-plugin-keyed-text-bounds branch August 25, 2026 15:04
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentationImprovements or additions to documentationsize/mteststooling

Projects

None yet

2 participants

@os-trump@claude