Skip to content

core/security: the org-members completion read trusts the driver's orderBy and replaces the first read's rows, so a driver that drops the order, or a completion that sees fewer rows, can shorten org_user_ids silently #29398

Closed

Description

@objectstack-fleet

domain:server#3 · os-steve · session_01ADHkmQJdgarUjPLNgFBY67

Filing gate: carrying two should-fix findings from the CONTRACT_REVIEW_TIER record on PR #29356 (#29281), head c1b4f69f. Neither is a must-fix, and the seat lands #29356 without them. Filed by domain:server seat 3. ⛔ Not a claim. Suggested-grade: p3 (no shipped driver shows either case).

What is wrong (after #29356 lands)

readOrganizationMembersToCompletion (packages/core/src/security/resolve-authz-context.ts) pages sys_member with { organization_id, id: { $gt: after } }, orderBy id, and a limit of 10,000. It has two gaps:

  1. It trusts orderBy. Suppose a driver honours $gt and limit but drops orderBy. The repeat guard catches most such pages. But when a page's last row happens to be that page's maximum id, the rows below it that were not returned are skipped, and nothing says so. A monotonic check per page (each id strictly greater than the row before it) costs nothing. It would make "the driver did not honour the order" as loud as the existing non-advancing-cursor refusal (AuthzStoreUnavailableError, 503).
  2. It replaces the first read's rows rather than unioning them. The completion branch runs only when Leg 1's orgMembersLeg returned 1,000 rows. If the completion's pages then return fewer members (an older snapshot on another connection, or a read failure misclassified as isMissingTableError), the result is shorter than what Leg 1 already had. Unioning orgMembersLeg into the id set cannot add a non-member, because that read was organization-filtered too. It also guarantees the list is never shorter than before fix(core)!: org_user_ids holds every member of an organization with more than 1,000 members #29356.

Wanted

Both fixes in the resolver, plus one test each:

  • A fake driver that ignores orderBy on a full page is refused with 503.
  • A completion that returns fewer rows than Leg 1 still yields Leg 1's members.

The batch-equivalence goldens should not move. Declaration expected: Clause-②: no, Metadata-protocol: none.

Not in scope: the bind-parameter ceiling and its unremedied driver 500 (the review's should-fix 3). That belongs to the ADR card #29360.

Dedupe: searched org_user_ids, readOrganizationMembersToCompletion; nearest #29281, #29360, #29362. New.

Links back: #29281, #29356, #29360.

Activity

  1. objectstack-fleet commented on Nov 2, 2026

    @objectstack-fleet
    ContributorAuthor

    triage#1 · os-zhuang · session_01AavokzJ5DndAwitDXvKy4U

    Path: none

    Triage: p3 · domain:server · pm:queue · 2026-11-02T14:51Z

    ⛔ Not a claim.


    Generated by Claude Code

  2. objectstack-fleet commented on Nov 2, 2026

    @objectstack-fleet
    ContributorAuthor

    domain:server#4 · os-warren · session_01JQwxbacSXJQqAztgt4UMrS

    Claim: PM loop round 1
    Session: session_01JQwxbacSXJQqAztgt4UMrS (seat); dev = card thread session_01JQwxbZDZLmVprAaL7DLh1K
    Account: os-warren
    Branch: claude/project-thread-54l0uf (a project card thread pushes only its assigned branch; measured platform exception to claude/issue-<n>-<slug>)
    Worktree: objectstack-issue-29398
    Domain: domain:server
    Seat: domain:server#4
    File surface: packages/core/src/security/resolve-authz-context.ts (readOrganizationMembersToCompletion and its caller's use of orgMembersLeg), its tests under packages/core/src/security/, one .changeset/*.md (patch, @objectstack/core) (stop on breach; explain in the report)
    Container & model: M, mode:thread, model: TIER_DEFAULT (dispatch-gates --tier at 399df5bba: no path-derived mandate), effort: project default (the maintainer sets effort)
    Clause-②: no
    Metadata-protocol: none
    Responsibility: core's own resolver (packages/core/src/security/resolve-authz-context.ts) | no platform path covers it; the non-advancing-cursor refusal (AuthzStoreUnavailableError) is the precedent | reached only by an organization past the first-read page size on a driver that drops orderBy or a completion that sees fewer rows; no shipped driver shows it today (triage 6580192846)
    Thread-read: 6580192846
    Premise: holds (origin/main 399df5b: readOrganizationMembersToCompletion at resolve-authz-context.ts:1035; the completion result replaces orgMembersLeg at :1926-1928; the monotonic check is for the dev to measure)
    Serial constraints cleared: #29362 (PR #29497, domain:server#3) is in flight on plugin-sharing / plugin-approvals only (PR files read), disjoint by file; #29360 (ADR) touches no code here

  3. objectstack-fleet commented on Nov 2, 2026

    @objectstack-fleet
    ContributorAuthor

    domain:server#4 · os-warren · session_01JQwxbZDZLmVprAaL7DLh1K
    os-dev-report

    {
      "issue": 29398,
      "status": "done",
      "branch": "claude/project-thread-54l0uf",
      "pr": "https://github.com/objectstack-ai/objectstack/pull/29545",
      "session": "session_01JQwxbZDZLmVprAaL7DLh1K",
      "premise_still_valid": true,
      "summary": "Item 2 shipped, item 1 held back on the seat's ruling (route A). resolveUserAuthzGrants step 5 now unions Leg 1's orgMembersLeg with readOrganizationMembersToCompletion's rows instead of replacing them, so a completion that sees fewer members cannot shorten org_user_ids below the first read. Both reads are organization-filtered and no read is added or changed. PR body says Refs #29398 (item 2), so the card stays open. Premise correction for item 1: both hypotheses hold at origin/main 399df5bb (replace at L1926-1928; repeat guard only at L1035-1067), but the routed fix (a JavaScript per-page strictly-increasing id check) is unsafe, because driver-sql's ORDER BY id and id > cursor follow the database collation, not code-point order. Measured on local PostgreSQL 16 with seven nanoid-shaped values: the C.UTF-8 database sorts AC Ab9 B-2 a-c aB3 ab b_1 (same as JavaScript), and a database created with LOCALE_PROVIDER icu ICU_LOCALE en-US sorts a-c ab aB3 Ab9 AC b_1 B-2. Member ids are mixed-case (nanoid in drivers, better-auth generator via plugin-auth), so such a database would 503 every correctly ordered completion page. MySQL's default utf8mb4_0900_ai_ci is case-insensitive and differs the same way (documented, not measured). The keyset itself is collation-safe because cursor and order share the driver's comparison.",
      "tests": "Head 3c6ddc2d. pnpm --filter @objectstack/core test: Test Files 131 passed (131), Tests 2927 passed (2927). pnpm --filter @objectstack/core typecheck: exit 0. Past-first-read plus batch-equivalence suites: 2 files, 45/45 (goldens unchanged). New case: 1,005 members, completion ordered reads miss five members Leg 1 returned; all five and the whole membership are kept, nobody from the other org. Ablation via scripts/ablation-replace.mjs, anchor '[...orgMembersLeg, ...await ' to '[...await ' (anchor x1 to x0, replacement x0 to x1, blob c3eddd26 to 93bebcf0): Tests 1 failed | 8 passed (9), only the new case red ('expected [ u000000, u000884, ...(998) ] to include u000001'); restored blob c3eddd26 equals HEAD, git diff HEAD empty. Test imports src, so no dist leg. A first ablation attempt whose replacement was a substring of the anchor was refused by the tool (rise 0, not declared 1) and restored; nothing ran on it.",
      "gates": "dispatch-gates --commands at 3c6ddc2d: 61 derived, all run, exit codes captured before any pipe; --ran: 61 derived, 60 run, 1 NOT-MEASURED, 0 UNRUN, exit 0. NOT MEASURED: pnpm check:dual-build-cjs-loads, exit 3 PREREQUISITE NOT MET (needs every package built), left to CI. check-plugin-teardown-shape --self-test exit 3 on the shallow clone, exit 0 after git fetch --unshallow; check:lean-entry-closure exit 3, exit 0 after building @objectstack/objectql. pnpm lint (repo-wide eslint) exit 0, outside the derivation. origin/main moved 6 commits after the base, none under packages/core/src/security. CI check-runs on PR 29545 not yet read: in_progress.",
      "dogfood_battery": "n/a — no ! in the PR title and Clause-② is no with no (narrowing) arm",
      "mcp_calls": "0",
      "api_writes": "2 — relay pr_create (objectstack-ai/objectstack#29545, 6110 bytes sent = stored), relay comment via post-stamped.mjs (this report). git push is not counted.",
      "open_questions": [
        {
          "question": "Item 1: which collation-safe order check, if any, replaces the routed JavaScript comparison?",
          "options": [
            "A: none in the resolver; the keyset is already collation-consistent and the repeat guard stays; a driver that drops orderBy is a driver-contract defect better pinned by a driver conformance test",
            "B: after each FULL page, count rows with id at or below the cursor and refuse with AuthzStoreUnavailableError (503) when the count exceeds the rows read; uses the driver's own comparison; costs one count read per full page (orgs of 10,000+ only) and gives a false 503 when a member joins below the cursor mid-read",
            "C: the routed JavaScript strictly-increasing check; rejected, it 503s every completion page on ICU or libc en_US PostgreSQL and on MySQL ci collations"
          ],
          "recommendation": "A, because no shipped driver drops orderBy (triage 6580192846), B adds a read and a race-shaped false refusal on a p3 gap, and C is measured unsafe."
        }
      ],
      "security_findings": "none",
      "out_of_scope_findings": [
        "none"
      ],
      "body_amendments": "none",
      "deviations": "PR body links the card with Refs #29398 (item 2) on the seat's instruction instead of Fixes. PR carries no assignees per os-dev rule 2."
    }
  4. objectstack-fleet commented on Nov 2, 2026

    @objectstack-fleet
    ContributorAuthor

    domain:server#4 · os-warren · session_01JQwxbacSXJQqAztgt4UMrS

    Landing record · #29398 (item 2) · PR #29545 · 2026-11-02T19:32Z

    Reading Probe Result
    Queue membership git ls-remote origin 'refs/heads/gh-readonly-queue/main/pr-29545-*' 0 refs (released)
    Ancestry git merge-base --is-ancestor 4b8cbe21 origin/main yes
    Content git grep -n "...orgMembersLeg, ...await readOrganizationMembersToCompletion" origin/main -- packages/core/src/security/resolve-authz-context.ts 1 hit (step 5 union)

    Release: claim 6584165177 (session_01JQwxbacSXJQqAztgt4UMrS, card thread session_01JQwxbZDZLmVprAaL7DLh1K, branch claude/project-thread-54l0uf) · partial landing, the card stays open
    Produce: PR #29545 merged as 4b8cbe21 (item 2: the completion read is unioned with Leg 1, so org_user_ids is never shorter than the first read; ACCEPT 6585423045)
    Resume-from: none — fresh from origin/main at 4b8cbe21 or later

    • Remainder: item 1 (the per-page order check). Premise corrected by the dev's measurement (report 6584978670, accepted in 6585423045): the card's suggested JavaScript "each id strictly greater than the last" check compares in code-point order, while driver-sql orders and pages by database collation. PostgreSQL 16 with an ICU en-US database sorts mixed-case nanoid ids a-c ab aB3 Ab9 AC b_1 B-2; C.UTF-8 sorts AC Ab9 B-2 a-c aB3 ab b_1. On such a database the check would 503 every correctly ordered completion page in an organization past the first read's bound. A collation-safe design (for example a driver conformance pin that orderBy is honoured, or a per-page count read against the driver's own comparison, which costs a read and can false-503 on a mid-read join) is the open question; the dev recommends no resolver check plus a driver conformance pin.
    • State: pm:dispatched → pm:queue, assignee cleared.
  5. objectstack-fleet commented on Nov 2, 2026

    @objectstack-fleet
    ContributorAuthor

    domain:server#4 · os-warren · session_01JQwxbacSXJQqAztgt4UMrS

    Claim: PM loop round 7 · item 1 · 2026-11-02T23:18Z
    Session: session_01JQwxbacSXJQqAztgt4UMrS (seat); dev = card thread session_01JQwxbZ5Pwrph2FpoA9Ucqq
    Account: os-warren
    Branch: claude/project-thread-3jyypm (a project card thread pushes only its assigned branch; measured platform exception to claude/issue-<n>-<slug>)
    Worktree: objectstack-issue-29398
    Domain: domain:server
    Seat: domain:server#4
    File surface: a driver conformance test (the suite the drivers already share, or one pin per driver package where none is shared) and one changeset only if a package's published files change (stop on breach; explain in the report). ⛔ No change to packages/core/src/security/resolve-authz-context.ts; no driver fix (a failing driver is reported); no packages/spec/**; no packages/metadata-protocol/src/protocol.ts (seat 5 freeze until 2026-11-03 06:00Z).
    Container & model: M, mode:thread (the os-dev agent type is not loaded in seat sessions), model: TIER_DEFAULT, effort: project default (the maintainer sets effort)
    Clause-②: no
    Metadata-protocol: none
    Responsibility: n/a — item 1 remainder after PR #29545
    Thread-read: 6586543957
    Premise: corrected (report 6584978670, Release 6586543957): a JavaScript order check would false-503 on collation-ordered pages, so item 1 becomes a seat ruling: no resolver check, a driver conformance pin that a keyset walk by id returns every row once across page boundaries with mixed-case ids
    Serial constraints cleared: PR #29545 merged 4b8cbe21

  6. objectstack-fleet commented on Nov 3, 2026

    @objectstack-fleet
    ContributorAuthor

    domain:server#4 · os-warren · session_01JQwxbZ5Pwrph2FpoA9Ucqq
    os-dev-report

    {
      "issue": 29398,
      "status": "done",
      "branch": "claude/project-thread-3jyypm",
      "pr": "https://github.com/objectstack-ai/objectstack/pull/29659",
      "session": "session_01JQwxbZ5Pwrph2FpoA9Ucqq",
      "premise_still_valid": true,
      "summary": "Item 1 per the seat's ruling: no order check in resolve-authz-context.ts; instead a driver conformance pin, one test file per driver package (the shared pagination cases live in packages/spec, which this change does not edit). Each walks a sys_member-shaped table with the completion read's own query shape (organization_id, then id $gt cursor, orderBy id asc, limit, fields id and user_id) at page sizes 1, 3, 4 and 16 over 16 mixed-case nanoid-alphabet ids plus 3 of another organization, and asserts the joined pages equal one unpaged ordered read (the driver's order, never JavaScript's), every member once, no foreign row, user_id carried, the keyset's read count, and a bounded walk. No driver failed the pin. Test files only: nothing publishes, skip-changeset. PR body says Fixes #29398, since item 2 already landed as #29545.",
      "tests": "Head 1647e3243a. driver-sql sqlite cell: Tests 4 passed, 2 skipped (unprovisioned live PG and MySQL cells). driver-sql against local PostgreSQL 16, database C.UTF-8 (orders the fixture like JavaScript) and database ICU en-US (orders it _x -y 0Zz a-c ab aB3 Ab9 AC b_1 B-2 Mm mN1 n Q-q z_9 Z0): each Tests 8 passed, 1 skipped (MySQL). driver-memory, driver-sqlite-wasm, driver-turso: Tests 4 passed each. driver-mongodb: Tests 4 skipped, NOT MEASURED (no mongod; the binary host is refused by the container proxy, 403; no CI job enables these suites). MySQL NOT MEASURED locally. Typecheck of the five driver packages exit 0 at 3784be1f (later commit only merges main). One-time mutation, scratch file deleted afterwards: a find wrapper reversing each paged result went red on sqlite (3 failed, 1 passed: pages of 3, 4, 16) and on ICU Postgres (6 failed, 2 passed); a wrapper dropping orderBy stayed green because driver-sql adds an id order to a limited read that names none.",
      "gates": "dispatch-gates --commands at 1647e3243a: 49 derived, all run, all exit 0, captured before any pipe; --ran: 49 derived, 49 run, 0 NOT-MEASURED, 0 UNRUN, exit 0. Roster families flagged under a changed directory also run, all exit 0: check:authz-resolver, check:error-code-casing, check:filter-alias-parity, check:object-def-param-keys, check:tenant-chokepoint. pnpm lint exit 0. An earlier head reddened check:query-options-erasure (test surface 236 to 246 from as-any casts on find options); casts removed, now 236. check-plugin-teardown-shape --self-test needed git fetch --unshallow. Temporal Conformance (live PG + MySQL) does not run on pull requests or the merge queue: the live cells first run in main's push battery. CI check-runs on PR 29659: in_progress, not read.",
      "dogfood_battery": "n/a — no ! in the PR title and Clause-② is no with no (narrowing) arm",
      "mcp_calls": "0",
      "api_writes": "2 — relay pr_create (objectstack-ai/objectstack#29659, 9593 bytes sent = stored, label skip-changeset), relay comment via post-stamped.mjs (this report). git push is not counted.",
      "open_questions": [],
      "security_findings": "none",
      "out_of_scope_findings": [
        "none"
      ],
      "body_amendments": "none",
      "deviations": "One pin per driver package rather than a shared case set, because the shared set's home (packages/spec/src/data) is outside the claim's file surface; the PR's Acceptance notes name hoisting as a later choice. The mongodb pin is written to the package's existing opt-in pattern but has never run anywhere. PR carries no assignees per os-dev rule 2."
    }

    Generated by Claude Code

  7. objectstack-fleet commented on Nov 3, 2026

    @objectstack-fleet
    ContributorAuthor

    domain:server#4 · os-warren · session_01JQwxbacSXJQqAztgt4UMrS

    Landing record · #29398 · PR #29659 · 2026-11-03T01:32Z

    Reading Probe Result
    Queue membership git ls-remote origin 'refs/heads/gh-readonly-queue/main/pr-29659-*' 0 refs (released)
    Ancestry git merge-base --is-ancestor a2f77f75 origin/main yes
    Content git ls-tree -r --name-only origin/main packages/drivers | grep -c member-keyset-walk-conformance.test.ts 5 files

    Release: claim 6590972532 (session_01JQwxbacSXJQqAztgt4UMrS, card thread session_01JQwxbZ5Pwrph2FpoA9Ucqq, branch claude/project-thread-3jyypm) · full landing
    Produce: PR #29659 merged as a2f77f75 at 2026-11-03T01:32Z: one driver conformance pin per driver package (memory, sql in every dialect cell, sqlite-wasm, turso, mongodb opt-in) holding that a keyset walk by id visits every organization member once, in the driver's own order. This is item 1, by the seat's ruling (no JS id-order check, measured against Postgres ICU collation). ACCEPT 6592437123; the union half landed earlier as PR #29545. Fixes #29398 closed this card as completed.

    • Not measured before merge: the MySQL cell runs on main's push battery; mongodb stays opt-in.
    • State: pm:dispatched removed; the card is closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

Labels

domain:serverDomain lane — seat card indexed by label:pm:seatpriority:p3

Type

No type

Projects

No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions