UN-4399 [FEAT] Organisation capability permissions API by ritwik-g · Pull Request #2344 · Zipstack/unstract

ritwik-g · 2026-10-14T23:21:15Z

What

PR-1 of UN-4399 (Frontend v1 PB-T04): organisation capability permissions. An org admin chooses, per capability, whether any member or only admins may use it.

Storage. Seven capabilities, each a restrict_* boolean on Organization.
- The two existing fields, restrict_llm_adapter_creation and restrict_connector_creation, are the storage for the first two capabilities.
- Five new fields are added (migration account_v2/0008), with defaults that match today's behaviour.
Read and write API. GET/PATCH /api/v1/unstract/{org}/organization/capabilities/.
- GET returns every capability with its setting and allowed_for_me. Any member may call it.
- PATCH accepts any subset of keys and validates everything before saving. Organisation admins only.
Legacy sync. organization/settings is unchanged and stays in step by construction, because both endpoints read and write the same fields.
Audit. Each changed capability records one capability.changed event through the PB-T01 recorder, whichever endpoint made the change.
Enforcement (the capabilities that need no product decision):
- LLM adapter creation, connector creation and other-adapter creation (all non-LLM types).
- Deleting workflows, API deployments and pipelines. This adds a check next to IsOwner, so with any_member nothing changes, and with admins_only owners who are not admins are refused.
Contract. backend/tenant_account_v2/CAPABILITIES.md (endpoint, payloads, errors, TypeScript interfaces, what allowed_for_me means) plus @extend_schema, following the PB convention.

Stacked on #2342 (PB-T01). This PR's base is that branch, so the diff shows only PB-T04.

Why

Frontend v1 PB-T04 (gap G-04). The Platform › Permissions tab (P3-T12) consumes this contract.

How

tenant_account_v2/capabilities.py holds the registry: Capability, CapabilitySetting, a spec per capability, is_allowed, enforce_capability and the RequiresCapability DRF permission.
Service accounts are never subject to a capability check, so every endpoint keeps its existing exemptions.
"Admin" uses the existing OrganizationMemberService.is_user_organization_admin.

Decide-and-flag choices (tracker `decisions/D-PB-T04-*`; UN-4399 comment 47083)

Unenforced capabilities refuse any_member. Until PR-2 lands, PATCH refuses any_member for manage_api_keys, manage_members and change_platform_defaults. Their endpoints are still admin-only, so storing that value would make allowed_for_me misleading.
Labels and descriptions are my wording, not the design's. Labels use the spec's capability names; I wrote the short descriptions. Figma P12 was not accessible, so all the copy lives in capabilities.py and is a one-place swap.
One audit event, with PB-T01's action key. Changes are recorded as capability.changed, the key PB-T01 already registered. The legacy organization/settings PATCH now records capability.changed instead of platform_setting.changed. PB-T01's README and its test are updated in this branch to match.
Per-user default adapters are not governed. The per-user adapter/default_triad stays open to members, as in the dossier's default.

Can this PR break any existing features. If yes, please list possible items. If no, please explain why. (PS: Admins do not merge the PR without this section filled)

With default settings there is no change in behaviour: every new field defaults to what the code does today.
The two legacy refusals keep their exact messages. The LLM and connector checks were moved onto the registry with the same logic.
Deletion gains an extra check, but it is a no-op unless an admin sets delete_workflows_and_deployments to admins_only.
Behaviour change: organization/settings PATCH now records capability.changed in the audit log, not platform_setting.changed. That only matters if UN-4398 [FEAT] Platform audit log: event store and admin read API #2342 is merged first.

Database Migrations

account_v2/0008_organization_capability_settings adds five boolean columns to organization, each with a default.

Env Config

None.

Relevant Docs

backend/tenant_account_v2/CAPABILITIES.md
The spec is in PT 1637187626 (PB-T04).

Related Issues or PRs

UN-4399
Stacked on UN-4398 [FEAT] Platform audit log: event store and admin read API #2342 (UN-4398).
PR-2 (enforcement for API keys, members and platform defaults) waits on item 2 of the UN-4257 comment 47060 answer.
The rotate-endpoint hook for PB-T05 is proposed on UN-4360.

Dependencies Versions

None.

Notes on Testing

backend/tenant_account_v2/test_capabilities.py covers:
- the registry;
- GET defaults and allowed_for_me for admin, member and service account;
- PATCH permissions, validation and saving nothing on bad input;
- unenforced capabilities;
- legacy sync in both directions;
- the audit events;
- the full enforcement matrix: four enforced capabilities × both settings × {admin, member, service account}, through the real connector and workflow views;
- deletion gating for API deployments and pipelines.
makemigrations --check is clean.
The unit-backend and integration-backend rig tiers were run locally, because CI skips draft PRs. Results are in a comment below.

Screenshots

None; this is backend only.

Checklist

I have read and understood the Contribution Guidelines.

🤖 Generated with Claude Code

sonarqubecloud · 2026-10-14T23:22:41Z

Quality Gate passed

Issues
2 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

ritwik-g

Self-review round 1 — `unstract:review-deep` (INITIAL) at tip `5b1da17de`

Review: review-deep. The PR adds a migration, a permission class and a new endpoint. It is stacked on #2342, so the base is feat/UN-4398-platform-audit (merge base b0fa57faa). Scope is 15 files, 909 lines; the file count matches GitHub's.

Signals: lenses 4, 5, 7, 13 and 16 raised, from account_v2/0008, RequiresCapability and the destroy permission lists, the new endpoint and serializers, the new tests, and CAPABILITIES.md and the README.

Verdict — REQUEST CHANGES

Summary — Critical: 0 · High: 1 · Medium: 6 · Low: 5 · Lenses run: 19/19 (lenses 12, 14 and 19 are N/A).

Security deep-dive: done. No findings at confidence ≥ 8. The one candidate (filtered at 3/10) is filed as #3.

boundary-sweep: 2 sinks traced, clean. Those are the audit event payloads and the refusal messages; neither carries secrets.

Lens checklist

Spec & intent — Clean (code-reviewer)
Architectural fit & precedent — Clean (code-reviewer)
Correctness & edge cases — See findings #3, #4
Security — See findings #3
Data integrity & migrations — See findings #2
Concurrency — Clean (infra agent: update_fields scoped per key)
API & contract compatibility — See findings #8, #9 (orchestrator consumer search: organization/settings response unchanged; no consumers of the new endpoint)
Reliability & resilience — Clean (infra agent)
Performance & cost — Clean (code-reviewer: one org query per delete; admin lookup memoized)
Observability — Clean (code-reviewer)
Operational safety — See findings #2
LLM/agent-specific — N/A — no LLM code
Testing — See findings #1, #7, #11
Dependencies & build — N/A — none
Code quality — See findings #12 (orchestrator: pre-commit passed)
Doc & comment accuracy — See findings #5, #6, #10
Cross-document rule consistency — See findings #5 (orchestrator: README vs CAPABILITIES.md vs code)
On-prem deployment experience — See findings #2 (an on-prem image downgrade hits the same NOT NULL insert)
Scaling & workload lifecycle — N/A — none

Open questions for the author

Does the deploy pipeline ever roll back the backend image without reversing migrations? This affects #2, which is fixed either way.

Assumptions made

Platform-key sessions can reach organization/capabilities/.

cost: agents=8 wall=12:00 lenses-examined=16 of 19 files=15 lines=909

ritwik-g

Self-review round 2 — `unstract:review-deep` (FOLLOWUP) at tip `10969981d`

Review: review-deep. The PR carries a migration and an authz change.

Delta: 5b1da17de..10969981d (162 lines). There is no cold read this round because the delta is under 200 lines.

Verdict — REQUEST CHANGES

Summary — Critical: 0 · High: 1 · Medium: 3 · Low: 2 · Lenses run: 19/19 (lenses 12, 14 and 19 are N/A).

Prior context fetched — followup-fetch.sh exited 0. "Since" boundary: the previous review's SHA, 5b1da17de.

Scope change — NO

Security deep-dive: done. No findings at confidence ≥ 8; not degraded.

Boundary sweep: 2 sinks traced, clean.

Prior findings status

#1 [High] destroy ownership untested → RESOLVED. The non-owner case and an IsOwner presence check are added; mutants killed.
#2 [Medium] no database defaults on the new columns → RESOLVED. RunSQL SET DEFAULT, verified on Postgres 15. The test gap is #16.
#3 [Medium] adapter type change on update → RESOLVED. The new type is enforced; the allowed side is untested (#15).
#4 [Medium] service accounts on unenforced capabilities → NOT RESOLVED. The fix overshot; see #13.
#5, #6, #7, #9, #10, #11 → RESOLVED.
#8 [Low] enum names → NOT RESOLVED. The override has no effect; see #14.
#12 [Low] "organisation" spelling → WAIVED. It is the spec's literal wording (dossier decision).

Lens checklist

Spec & intent — Clean (code-reviewer)
Architectural fit & precedent — Clean (code-reviewer)
Correctness & edge cases — See findings #13, #18
Security — Clean (orchestrator; security deep-dive done)
Data integrity & migrations — See findings #16
Concurrency — Clean (infra agent)
API & contract compatibility — See findings #14 (orchestrator: no consumers yet)
Reliability & resilience — Clean (infra agent)
Performance & cost — Clean (code-reviewer)
Observability — Clean (code-reviewer)
Operational safety — Clean (infra agent: the RunSQL defaults match the model; reverse is clean)
LLM/agent-specific — N/A — no LLM code
Testing — See findings #15, #16, #17
Dependencies & build — N/A — none
Code quality — Clean (orchestrator: pre-commit passed)
Doc & comment accuracy — See findings #13, #17
Cross-document rule consistency — See findings #13
On-prem deployment experience — Clean (infra agent: on-prem runs the same migrate; the defaults keep an image downgrade safe)
Scaling & workload lifecycle — N/A — none

Open questions for the author

Should repointing an existing LLM adapter to another provider count as "creating" one? For now this is documented as not governed (#18).

Assumptions made

None.

cost: agents=7 wall=09:00 lenses-examined=16 of 19 files=16 lines=1019

ritwik-g

Self-review round 3 — `unstract:review-deep` (FOLLOWUP) at tip `9ba8ed2dd`

Review: review-deep. The PR carries a migration and an authz change, so review-shallow's gate refuses it.

Pinned to b4dfa18fb: pre-commit.ci's ruff-format autofix, pushed during the round. It differs from the reviewed tip 9ba8ed2dd only by that formatting, which fixes #21.

Delta: 10969981d..9ba8ed2dd (136 lines, 5 files). There is no cold read this round because the delta is under 200 lines.

Verdict — REQUEST CHANGES

Summary — Critical: 0 · High: 0 · Medium: 1 · Low: 2 · Lenses run: 19/19 (N/A: 12, 14).

Prior context fetched — followup-fetch.sh fetched 6 of 6. "Since" boundary: 10969981d.

Scope change — NO

Security deep-dive: done. The find pass returned no candidates at confidence ≥ 6, so no filtering was needed; not degraded.

Boundary sweep: 0 new sinks in the delta. Clean.

Base fetch: ok. The count is verified against GitHub (16 files).

Prior findings status

#4, #8, #13–#18 → RESOLVED in 9ba8ed2dd. Each thread has a reply and is resolved. Mutants were killed for #4, #8, #15 and #16.

Lens checklist

Spec & intent — Clean (code-reviewer)
Architectural fit & precedent — Clean (code-reviewer)
Correctness & edge cases — Clean (code-reviewer)
Security — Clean (orchestrator, security deep-dive done, 0 findings)
Data integrity & migrations — Clean (dispatched infra agent: the static test pins both columns and values)
Concurrency — Clean (orchestrator: the delta adds no shared state)
API & contract compatibility — See findings #19, #20 (no consumers in frontend, unstract-cloud or clients)
Reliability & resilience — Clean (orchestrator: no I/O changed)
Performance & cost — Clean (code-reviewer)
Observability — Clean (code-reviewer)
Operational safety — Clean (orchestrator: settings-only enum-name change)
LLM/agent-specific — N/A — no LLM code
Testing — Clean (pr-test-analyzer, 5 mutants killed in a private worktree, no test weakened)
Dependencies & build — N/A — none changed
Code quality — See findings #21
Doc & comment accuracy — See findings #19
Cross-document rule consistency — See findings #19 (merge sweep: allowed_for_me statements agree except the schema help_text)
On-prem deployment experience — Clean (orchestrator: no chart, schema or migration change in the delta)
Scaling & workload lifecycle — Clean (orchestrator: no workload config)

cost: agents=7 wall=05:22 lenses-examined=17 of 19 files=5 lines=136

ritwik-g

Self-review round 4 — `unstract:review-deep` (FOLLOWUP) at tip `4ec12071e`

Review: review-deep. The PR carries a migration and an authz change, so review-shallow's gate refuses it.

Delta: 9ba8ed2dd..4ec12071e (36 lines, 4 files). That covers pre-commit.ci's b4dfa18fb and the round-3 fixes. There is no cold read this round because the delta is under 200 lines.

Verdict — APPROVE

Summary — Critical: 0 · High: 0 · Medium: 0 · Low: 0 · Lenses run: 19/19 (N/A: 12, 14).

Prior context fetched — followup-fetch.sh fetched 6 of 6. "Since" boundary: 9ba8ed2dd.

Scope change — NO

Security deep-dive: done. No candidates; filtering was not needed; not degraded.

Boundary sweep: 0 new sinks in the delta. Clean.

Base fetch: ok. The count is verified (16 files).

Prior findings status

#19 [Medium] allowed_for_me help_text → RESOLVED in 4ec12071e.
#20 [Low] KeyEnum → RESOLVED in 4ec12071e. A CapabilityKey override was added and a schema test asserts it; the mutant was killed.
#21 [Low] ruff-format → RESOLVED in b4dfa18fb (pre-commit.ci). The pinned pre-commit run is clean at 4ec12071e.

New findings

None.

Lens checklist

Spec & intent — Clean (code-reviewer)
Architectural fit & precedent — Clean (code-reviewer)
Correctness & edge cases — Clean (code-reviewer)
Security — Clean (orchestrator, security deep-dive done, 0 findings)
Data integrity & migrations — Clean (orchestrator, no migration or model in the delta)
Concurrency — Clean (orchestrator, no shared state)
API & contract compatibility — Clean (type-design-analyzer, full-backend schema has no collision, no consumers)
Reliability & resilience — Clean (orchestrator, no I/O)
Performance & cost — Clean (code-reviewer)
Observability — Clean (code-reviewer)
Operational safety — Clean (orchestrator, schema-name-only settings change)
LLM/agent-specific — N/A — no LLM code
Testing — Clean (pr-test-analyzer, 3 mutants killed, nothing weakened)
Dependencies & build — N/A — none changed
Code quality — Clean (orchestrator ran the pinned pre-commit, all hooks pass)
Doc & comment accuracy — Clean (comment-analyzer)
Cross-document rule consistency — Clean (orchestrator, help_text, docstrings and CAPABILITIES.md now agree)
On-prem deployment experience — Clean (orchestrator, no chart, schema or migration in the delta)
Scaling & workload lifecycle — Clean (orchestrator, no workload config)

cost: agents=5 wall=03:30 lenses-examined=17 of 19 files=4 lines=36

greptile-apps · 2026-10-15T00:00:22Z

via Greptile

Confidence Score: 5/5

[Critical impact] The PR appears safe to merge; both previous findings are fixed and no new blocking issue remains.

Summary

This PR adds seven organization capabilities, a GET/PATCH API, and checks for adapter creation, connector creation, and resource deletion.

Admins can set who may use each organisation capability.
Adapter and connector creation now follow their capability settings.
Admins can restrict deletes of workflows, deployments, and pipelines.

Diagram

%%{init: {'theme': 'neutral'}}%%
flowchart TD
  A[Capabilities PATCH] --> C[Validate requested settings]
  B[Legacy settings PATCH] --> C
  C --> D[Lock and read organization row]
  D --> E[Save requested fields]
  E --> F[Record changed capabilities]
  E --> G[Return updated settings]
  H[Creation or deletion request] --> I[Check capability setting]
  I --> J[Keep existing permission checks]

_{Reviews (2) · Last reviewed commit: "fix: lock the org row for capability wri..." · Reviewed by Greptile}

…n succeeds - Both settings endpoints read 'before' and save under select_for_update, so of two concurrent changes the second audits against the first's result. - The capability matrix creates a real connector and asserts 201 instead of treating any non-403 as allowed. Addresses Greptile review on #2344. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

Seven capabilities stored as restrict_* booleans on Organization (two legacy flags reused, five added with today's defaults), GET/PATCH organization/capabilities/ with effective permissions, capability.changed audit events from both endpoints, and enforcement for LLM, connector and other-adapter creation and for deleting workflows, deployments and pipelines. Key, member and platform-default enforcement is left for a follow-up; those three accept only admins_only until then. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

Keep database defaults on the new columns for older images; enforce the creation capability when an update changes an adapter's type; report unenforced capabilities as admin-only for service accounts; test ownership on delete and adapter creation through the endpoint; contract and README corrections; one CapabilitySetting enum in the schema. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

- allowed_for_me reports only what the setting allows; service accounts are never governed by a setting, endpoint rules still apply (round-1 change overshot: full_access keys can rotate). - Shared CAPABILITY_SETTING_VALUES so the schema enum override matches. - Tests: same-type adapter edit stays allowed, GET/PATCH 404 envelope, migration database defaults equal model defaults, schema enum name. - Docs: adapter edits not governed; 403-before-400 on type change; 404 on GET. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

for more information, see https://pre-commit.ci

- allowed_for_me schema help_text names the endpoint's own rules (admin checks, platform-key permission level, ownership), matching CAPABILITIES.md. - Publish the capability key enum as CapabilityKey, as the contract names it. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

…n succeeds - Both settings endpoints read 'before' and save under select_for_update, so of two concurrent changes the second audits against the first's result. - The capability matrix creates a real connector and asserts 201 instead of treating any non-403 as allowed. Addresses Greptile review on #2344. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

sonarqubecloud · 2026-10-16T07:37:59Z

Quality Gate passed

Issues
1 New issue
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

ritwik-g commented Oct 14, 2026

View reviewed changes

Comment thread backend/tenant_account_v2/serializer.py

Comment thread backend/tenant_account_v2/serializer.py Outdated

Comment thread backend/tenant_account_v2/test_capabilities.py Outdated

ritwik-g commented Oct 14, 2026

View reviewed changes

ritwik-g marked this pull request as ready for review October 14, 2026 23:56

greptile-apps Bot reviewed Oct 15, 2026

View reviewed changes

Comment thread backend/tenant_account_v2/views.py Outdated

Comment thread backend/tenant_account_v2/test_capabilities.py Outdated

ritwik-g requested review from Deepak-Kesavan and johnyrahul October 16, 2026 07:14

ritwik-g and others added 6 commits October 16, 2026 13:05

[pre-commit.ci] auto fixes from pre-commit.com hooks

183598a

for more information, see https://pre-commit.ci

ritwik-g force-pushed the feat/UN-4399-capability-permissions branch from a8bdbd4 to 6958f22 Compare October 16, 2026 07:36

ritwik-g added this pull request to stack #2351 October 16, 2026 07:36

Conversation

ritwik-g commented Oct 14, 2026

What

Why

How

Decide-and-flag choices (tracker decisions/D-PB-T04-*; UN-4399 comment 47083)

Can this PR break any existing features. If yes, please list possible items. If no, please explain why. (PS: Admins do not merge the PR without this section filled)

Database Migrations

Env Config

Relevant Docs

Related Issues or PRs

Dependencies Versions

Notes on Testing

Screenshots

Checklist

Uh oh!

sonarqubecloud Bot commented Oct 14, 2026

Quality Gate passed

Uh oh!

ritwik-g left a comment

Choose a reason for hiding this comment

Self-review round 1 — unstract:review-deep (INITIAL) at tip 5b1da17de

Lens checklist

Open questions for the author

Assumptions made

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

ritwik-g left a comment

Choose a reason for hiding this comment

Self-review round 2 — unstract:review-deep (FOLLOWUP) at tip 10969981d

Prior findings status

Lens checklist

Open questions for the author

Assumptions made

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

ritwik-g left a comment

Choose a reason for hiding this comment

Self-review round 3 — unstract:review-deep (FOLLOWUP) at tip 9ba8ed2dd

Prior findings status

Lens checklist

Uh oh!

Uh oh!

Uh oh!

Uh oh!

ritwik-g left a comment

Choose a reason for hiding this comment

Self-review round 4 — unstract:review-deep (FOLLOWUP) at tip 4ec12071e

Prior findings status

New findings

Lens checklist

Uh oh!

greptile-apps Bot commented Oct 15, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Confidence Score: 5/5

Summary

Diagram

Uh oh!

Uh oh!

Uh oh!

sonarqubecloud Bot commented Oct 16, 2026

Quality Gate passed

Uh oh!

Reviewers

Assignees

Labels

Decide-and-flag choices (tracker `decisions/D-PB-T04-*`; UN-4399 comment 47083)

Self-review round 1 — `unstract:review-deep` (INITIAL) at tip `5b1da17de`

Self-review round 2 — `unstract:review-deep` (FOLLOWUP) at tip `10969981d`

Self-review round 3 — `unstract:review-deep` (FOLLOWUP) at tip `9ba8ed2dd`

Self-review round 4 — `unstract:review-deep` (FOLLOWUP) at tip `4ec12071e`

greptile-apps Bot commented Oct 15, 2026 •

edited

Loading