Skip to content

docs(project): plan for async permission propagation and RBAC hardening#5144

Draft
rschlaefli wants to merge 3 commits into
v3from
permission-propagation-plan
Draft

docs(project): plan for async permission propagation and RBAC hardening#5144
rschlaefli wants to merge 3 commits into
v3from
permission-propagation-plan

Conversation

@rschlaefli

Copy link
Copy Markdown
Member

What This Adds

This PR adds the implementation plan for reworking permission propagation and hardening the RBAC/sharing check path — docs only, no code changes:

  1. project/2026-07-07-async-permission-propagation-plan.md: evidence-backed investigation of the current Permission/DerivedPermission model, the synchronous recursive recompute bottleneck (14 sharing mutations with { timeout: 60000 } transactions), and four verified auth-check findings.
  2. An 8-slice implementation plan: perf baseline → check-path hardening → set-based SQL recompute → synchronous fail-closed revoke fast-path → transaction shrink (grants only) → async grant cascade via Hatchet → reconciliation cron → docs. Execution order 3 → 6 → 4 is deliberate: the revoke fast-path must land before cascades leave the transaction, otherwise a crash creates a fail-open window.
  3. A review of the earlier concept [CONCEPT] New suggested approach for asynchronous permission propagation #4808 (transaction-concept): its diagnosis and invariants are adopted (fail-closed DerivedPermission-only checks, revoke > grant priority, idempotency keys); its PendingPermissionOperation queue mechanism is not — it reschedules the same O(users x hierarchy) per-row work instead of reducing it, and its infra options predate Hatchet in this repo. [CONCEPT] New suggested approach for asynchronous permission propagation #4808 stays open for reference per maintainer decision.

How It Was Built

  • Investigation by four scoped subagents (data model, resolver enforcement, async infra + UX, PR [CONCEPT] New suggested approach for asynchronous permission propagation #4808 deep review); load-bearing claims spot-checked against the worktree.
  • Two plan-review rounds, findings integrated inline (see plan's "Independent Plan Review" section): internal adversarial review (12 findings accepted) and external agy/Gemini review approved by the maintainer (8 accepted, 1 rebutted with evidence).

Important Details

  • changeElementStatus staying READ-gated is a confirmed intentional design (reviewers move element status without write permissions) — recorded in the plan as an invariant to document, not a gap to fix.
  • Security-relevant findings in the plan (e.g. checkAccess empty-array pass-through) are described at the design level; hardening is Slice 2 of the follow-up implementation.

Branch Coverage

  • Base: v3
  • Head: bfdf8c20c
  • Reviewed: 2 commits (611494666 initial plan, bfdf8c20c review-round integration), diff: 1 file, +187 lines, docs only.

Review Focus

  • Sync/async split semantics: revokes and downgrades stay synchronous fail-closed; only grants become eventually consistent (Slice 6 classification table).
  • Slice 3 introduces the repo's first raw SQL ($executeRaw) — guardrails: parameterized-only, permanent equivalence test as schema-drift tripwire, per-module ON CONFLICT targets on the shared 8-constraint DerivedPermission table.
  • Whether the open verification items (R4.5/R4.6: child creation/reparenting, group admin demote no-op, group ownership transfer) should block earlier slices.

Verification

Current head:

  • pnpm run check:all + pnpm run build -> pass (pre-commit and pre-push hooks, docs-only change).

Not run:

  • No runtime verification — plan document only.

Security / Privacy

  • Review: plan content covers permission-model security semantics; two review rounds included security-focused prompts. Mandatory $security-review gate is planned before the implementation PR, not this docs PR.
  • Sensitive data: none — public-repo file paths and design discussion only.

Blocking Before Merge

  • Maintainer agreement on the slice list and the sync/async classification table (this PR is the decision record).

Follow-Up After Merge

  • Execute Slice 1 (perf baseline + instrumentation) on a follow-up branch/PR referencing this plan.
  • Separate audit of response-api standard-mode authentication (plan R4.4) — out of scope here.

@coderabbitai

coderabbitai Bot commented Jul 7, 2026

Copy link
Copy Markdown

Important

Review skipped

Draft detected.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 036626df-692f-4e21-bfdc-42e84cd5ebf2

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@greptile-apps

greptile-apps Bot commented Jul 7, 2026

Copy link
Copy Markdown

Confidence Score: 5/5

This looks safe to merge.

  • No blocking issues found in the changed document.
  • The plan names the main permission safety constraints instead of changing runtime behavior.

Important Files Changed

Filename Overview
project/2026-07-07-pr-5144-async-permission-propagation-plan.md Adds a detailed implementation plan for permission propagation performance work and RBAC hardening, with no executable changes.

Reviews (1): Last reviewed commit: "docs(project): link plan to PR 5144" | Re-trigger Greptile

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

1 participant