Skip to content

fix(example-crm): bind sales positions to crm_sales_user permission set - #8167

Merged
os-zhuang merged 2 commits into
mainfrom
claude/issue-8060-crm-bind-positions-to-permission-set
Aug 12, 2026
Merged

fix(example-crm): bind sales positions to crm_sales_user permission set#8167
os-zhuang merged 2 commits into
mainfrom
claude/issue-8060-crm-bind-positions-to-permission-set

Conversation

@os-zhuang

Copy link
Copy Markdown
Contributor

Fixes#8060

What

examples/app-crm declared three positions (sales_rep, sales_manager,
finance_approver) and a crm_sales_user permission set granting CRUD on 5
CRM objects, but nothing ever bound them together — no
sys_position_permission_set seeding, and crm_sales_user was not
isDefault. Any user assigned one of the three positions therefore resolved
only the platform everyone baseline and was 403'd on every CRM object.

This mirrors examples/app-showcase/src/security/bind-position-sets.ts
exactly, per the triage ruling: a new
examples/app-crm/src/security/bind-position-sets.ts binds the three
positions to crm_sales_user imperatively on kernel:bootstrapped (a
declarative seed can't do this — the seed loader runs before the security
bootstrap creates the sys_position/sys_permission_set rows), wired via a
new onEnable export in objectstack.config.ts. crm_sales_user is
deliberately not marked isDefault — that would auto-bind it to the
everyone anchor and grant every user the CRM permissions, changing the
example's security story rather than completing it (explicitly ruled out).

Measured (predict-then-mutate + reverse verification)

Reproduced the issue's own acceptance test with
objectstack verify --app examples/app-crm/objectstack.config.ts --rls:

Before (confirmed matches the issue exactly):

declared=3 [sales_rep,sales_manager,finance_approver] ran=3 notRun=[]
sales_rep proven=0 unproven=6 (probeBlocked=6)
sales_manager proven=0 unproven=6 (probeBlocked=6)
finance_approver proven=0 unproven=6 (probeBlocked=6)
totals: probeBlocked=18, proven=6 (base persona only)

After, per persona: objects:6, probeBlocked:1, memberVisible:5.
probeBlocked (no object grant at all — the by-id-write class never
exercised) drops from 18/18 → 3/18. The one remaining probe-blocked object
per persona is crm_opportunity_line_item, which crm_sales_user does not
grant at all — a separate, pre-existing gap (filed as #8164, out of this
card's scope). Zero RLS holes introduced or found, in either state.

Reverse verification: disabled the onEnable wiring, re-ran — readout
returned to exactly the pre-fix 18/18 probeBlocked baseline, confirming the
hook (not something else) is the lever. Re-enabled and re-confirmed the fixed
readout before pushing.

A caveat on "proven": the report's strict proven field (⇒
rls-consistent) stayed at 0 for the position personas even after the fix —
not a defect of this change. All 5 now-granted CRM objects declare
sharingModel: 'public_read_write' ("everyone reads+writes"), so once the
object gate opens, the record is visible to any authenticated holder by
design — the report classifies that as member-visible (inconclusive), which
is correct and expected for this app, not rls-consistent. Zero holes is the
signal that matters: no security regression.

Sharing rules (PM mechanism assumption #2)

The three sharedWith: { type: 'position', value: … } sharing rules do
not start mattering after this fix — measured, not assumed. They widen
record visibility on top of the object's OWD baseline, but every CRM object
they target is already public_read_write (org-wide read+write for anyone
holding the object grant), so there is nothing left to widen. The sharing
rules remain inert on top of an already-maximal baseline; this is a property
of the objects' declared sharingModel, unrelated to the binding fix.

Out of scope

Tests

  • objectstack verify --app examples/app-crm/objectstack.config.ts --rls
    (see above) — the issue's own acceptance test, run before/after/reverted.
  • cd examples/app-crm && npx vitest run — 27/27 passing (unchanged; no test
    file added, following the showcase's own precedent of proving the binding
    via the live verify --rls probe rather than a unit test).
  • cd examples/app-crm && npx tsc --noEmit — clean.
  • pnpm --filter '@objectstack/example-crm...' build — clean, only the
    pre-existing (now separately filed) security-master-detail-ungranted
    warning on crm_opportunity_line_item.
  • pnpm check:changeset-gate-self-tests, check:i18n-coverage (after
    rebuilding the showcase closure to clear a stale-dist false positive
    unrelated to this diff), check:objectui-changeset,
    check-changeset-no-major.mjs, check:nul-bytes — all green. Note
    check:objectui-pin-fresh / check-objectui-pin-fresh.mjs are red, but
    pre-existing and unrelated (ambient objectui pin staleness, independent of
    this diff's content).

Generated by Claude Code

…m_sales_user
The CRM example declared three positions and a crm_sales_user permission
set that never met -- no sys_position_permission_set seeding, and the set
was not isDefault (which would grant every user, not just these three).
Every persona assigned one of the three positions resolved only the
everyone baseline and was 403'd on every CRM object.
Mirrors examples/app-showcase/src/security/bind-position-sets.ts: binds
the three positions to crm_sales_user imperatively on kernel:bootstrapped
(cannot be a declarative seed -- the seed loader runs before the security
bootstrap creates the sys_position/sys_permission_set rows), wired via a
new onEnable export in objectstack.config.ts.
Measured with `objectstack verify --rls`: the three per-position probe
personas go from 18/18 probe-blocked to 3/18 (crm_opportunity_line_item,
ungranted by crm_sales_user -- filed separately as #8164, out of scope).
Zero RLS holes in either state. Reverse-verified: disabling the hook
restores the exact 18/18 pre-fix baseline.
Fixes#8060
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01PEVB6w7D7uCszR9Mw1BL73
@vercel

vercelBot commented Aug 12, 2026

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

1 Skipped Deployment
ProjectDeploymentActionsUpdated (UTC)
objectstackIgnoredIgnoredAug 12, 2026 7:13pm

Request Review

…k deferral
The workspace-root tsc program (tsconfig.json at repo root) has no dom/node
lib types. examples/app-crm/src/security/bind-position-sets.ts is pulled
into that program transitively via scripts/analytics-reconcile/app-crm.ts,
so the fallback branch's `setTimeout(...)` (unreachable in practice -- the
real runtime PluginContext always provides `.hook`, and no test exercises
this branch) was an undeclared global there: TS2304, +1 over the frozen
@objectstack/spec-monorepo DEBT entry (80).
Replaced with `void Promise.resolve().then(run)` -- a pure-ES microtask
deferral needing no ambient global, equivalent in this unreachable branch.
Verified in isolation (tsc --ignoreConfig under the root's exact compiler
options) and via the full `pnpm check:type-check-debt` re-measure: the
spec-monorepo entry is back to exactly 80.
Part of #8060
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01PEVB6w7D7uCszR9Mw1BL73
@os-zhuang
os-zhuang marked this pull request as ready for review August 12, 2026 19:24
@os-zhuang
os-zhuang added this pull request to the merge queueAug 12, 2026
Merged via the queue into main with commit 7e06f51Aug 12, 2026
25 checks passed
@os-zhuang
os-zhuang deleted the claude/issue-8060-crm-bind-positions-to-permission-set branch August 12, 2026 19:44
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentationImprovements or additions to documentationsize/mtooling

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[finding] The CRM example binds none of its 3 declared positions to crm_sales_user, so a user holding one is 403 on every CRM object

2 participants

@os-zhuang@claude