UN-4398 [FEAT] Audit resource create, delete and share events by ritwik-g · Pull Request #2350 · Zipstack/unstract

ritwik-g · 2026-10-16T07:09:52Z

What

Records resource actions in the platform audit log (UN-4398, PB-T01), stacked on #2342. Hari confirmed the scope in the v1 channel; see UN-4398 comment 47129.

Action	Recorded on
`resource.created`	create of an adapter, connector, workflow, API deployment, pipeline or Prompt Studio project
`resource.deleted`	delete of the same; a refused delete records nothing
`resource.sharing_changed`	`share/` on any shareable resource; only the axes that changed (user e-mails, group names, `shared_to_org`)
`resource.co_owner_added` / `resource.co_owner_removed`	`owners/` add and remove; co-owner e-mail in the summary

Target type: the resource's kind from SHAREABLE_RESOURCES, so it matches the share endpoints. On cloud, Agentic Studio projects and Look-Ups use the same share and co-owner endpoints, so their sharing is recorded too.
Summary: the adapter type (LLM, ...) or pipeline type (ETL, TASK) where there is one.
Out of scope: field-level edits (renames, settings).

How

platform_audit/resources.py:
- resource_ref reads type, id and name before a delete, since a deleted row has no pk.
- record_resource_event and record_sharing_change are best_effort wrappers over record_platform_event.
Share and co-owner recording sit in the two shared mixins, so they cover every host viewset.
Create and delete are recorded at each viewset after the change succeeds. Workflows and API deployments gain a small perform_destroy override.

Tests

platform_audit/tests/test_resource_events.py has 12 tests, all through the real viewsets:

create and delete per resource type;
a refused delete;
each share axis, and no-op shares;
the adapter share override;
co-owner add and remove;
org scoping (the event belongs to the resource's org, and another org's admin reads nothing).

Every emit site was mutation-checked: removing any one of the 12 create/delete calls, or the share and co-owner logic, fails a test. The touched apps' suites pass, except test_pdf_marker_inside_other_content_does_not_smuggle_a_file_through, which also fails on main.

Merge order

The base is feat/UN-4398-platform-audit (#2342). Merge after #2342.

🤖 Generated with Claude Code

ritwik-g

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

Review: review-deep. The diff touches permissions/, which trips the auth trigger, so review-shallow's gate refuses it. Findings are triaged by Ritwik's must-haves bar: only billing, data, security or a wrong operator action blocks; everything else is an accepted note.

Verdict — REQUEST CHANGES

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

Scope change — NO

Security deep-dive: done. No candidates, so filtering was not needed; not degraded. It found no change to any authorization decision.

Boundary sweep: 2 sinks traced (audit rows, and the best-effort failure logs), clean. Share recipients' e-mails go only into admin-only audit rows, as member events already do. Logs carry no message.

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

Accepted notes (recorded, not fixed, per the must-haves bar)

resource_ref is called outside best_effort before each delete. Nothing in the current registry can make it raise; this is hardening only.
Two concurrent share requests can leave a shared_to_org toggle unrecorded. The same accepted window exists for users and groups.
Adapter create: an IntegrityError from the later default save can leave the adapter row (pre-existing) with no event.
resource.created and resource.deleted carry empty changes; the rows don't say so.
TargetType enum members for the kinds are not added (target_type is free text).
Share recipients are not searchable; they live in changes, not the summary.
The co_owner_removed row omits "e-mail in summary".
The README doesn't say outright that cloud kinds get sharing and co-owner events only.
The org-scoping test can't tell the resource's org from the request's org (they are always equal in views).
The sharing test can't tell e-mail from username (fixture sets both alike).
resources.py docstring: "the share endpoints" don't use kind; "any other model" overstates create/delete.
The README summary rule conflicts with the co-owner summary (e-mail).
"ETL, TASK" reads as the full list of pipeline types.

Lens checklist

Spec & intent — See findings #1
Architectural fit & precedent — Clean (code-reviewer)
Correctness & edge cases — See findings #2 (silent-failure-hunter and code-reviewer, otherwise notes only)
Security — Clean (orchestrator, security deep-dive done with 0 findings, boundary sweep of 2 sinks)
Data integrity & migrations — Clean (infra agent, no migrations, events recorded after commit)
Concurrency — Clean (infra agent, one accepted note)
API & contract compatibility — See findings #1, #3 (type-design-analyzer, no consumers yet)
Reliability & resilience — Clean (infra agent, recording is best-effort)
Performance & cost — Clean (code-reviewer)
Observability — Clean (code-reviewer)
Operational safety — Clean (infra agent, revertable, no schema)
LLM/agent-specific — N/A — no LLM code
Testing — See findings #4 (pr-test-analyzer, 15 emit sites mutation-covered)
Dependencies & build — N/A — none
Code quality — Clean (orchestrator ran the pinned pre-commit, all hooks pass)
Doc & comment accuracy — See findings #2
Cross-document rule consistency — See findings #3
On-prem deployment experience — N/A — no chart, schema or settings
Scaling & workload lifecycle — N/A — no workload config

cost: agents=8 wall=12:02 lenses-examined=15 of 19 files=12 lines=514

ritwik-g

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

Review: review-deep, delta only (f0790242d..bedbcccab, 100 lines, 3 files); there is no cold read under 200 lines. The must-haves bar applies.

Verdict — REQUEST CHANGES

Summary — Critical: 0 · High: 0 · Medium: 2 · Low: 0 · Lenses run: 19/19 (N/A: 12, 14, 18, 19). There are 2 must-haves; 3 notes are accepted.

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

Scope change — NO

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

Boundary sweep: 1 new sink (the import event row: tool id and name only), clean.

Prior findings status

#1 → RESOLVED in bedbcccab (its failure-path test is #5).
#2, #3, #4 → RESOLVED in bedbcccab.

Accepted notes

The in-use connector test asserts >= 400, not 409.
create_connector hardcodes its name.
The cascade sentence reads as the only example (API deployment keys are covered elsewhere in the README).

Lens checklist

Spec & intent — See findings #6
Architectural fit & precedent — Clean (code-reviewer)
Correctness & edge cases — See findings #6
Security — Clean (orchestrator, security deep-dive done with 0 findings)
Data integrity & migrations — Clean (orchestrator, no model change)
Concurrency — Clean (orchestrator, no shared state)
API & contract compatibility — Clean (orchestrator, the Target types line now matches the registry)
Reliability & resilience — Clean (orchestrator, the emit is best-effort)
Performance & cost — Clean (code-reviewer)
Observability — Clean (code-reviewer)
Operational safety — Clean (orchestrator, no config)
LLM/agent-specific — N/A — no LLM code
Testing — See findings #5
Dependencies & build — N/A — none
Code quality — Clean (orchestrator ran the pinned pre-commit, all hooks pass)
Doc & comment accuracy — Clean (comment-analyzer, one note)
Cross-document rule consistency — Clean (orchestrator, README cascade and target types agree with the code)
On-prem deployment experience — N/A — no chart, schema or settings
Scaling & workload lifecycle — N/A — no workload config

cost: agents=4 wall=04:35 lenses-examined=15 of 19 files=3 lines=100

ritwik-g

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

Review: review-deep, delta only (bedbcccab..f43427d41, 64 lines, 2 files). The must-haves bar applies.

Verdict — APPROVE

Summary — Critical: 0 · High: 0 · Medium: 0 · Low: 0 · Lenses run: 19/19 (N/A: 12, 14, 18, 19). There are no must-haves; 1 note is accepted.

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

Scope change — NO

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

Boundary sweep: 1 new sink (the backup event row: id, name, and a summary with the source project name, same org), clean.

Prior findings status

#5 → RESOLVED in f43427d41.
#6 → RESOLVED in f43427d41.

Accepted notes

The sync-prompts test stubs the sync step, so moving the backup emit after the sync would survive. The current order is correct, and the general lens confirmed that no other user-reachable CustomTool create path remains.

Lens checklist

Spec & intent — Clean (code-reviewer)
Architectural fit & precedent — Clean (code-reviewer)
Correctness & edge cases — Clean (code-reviewer, every create_tool_from_import_data caller now records)
Security — Clean (orchestrator, security deep-dive done with 0 findings)
Data integrity & migrations — Clean (orchestrator, no model change)
Concurrency — Clean (orchestrator, no shared state)
API & contract compatibility — Clean (orchestrator, no contract change)
Reliability & resilience — Clean (orchestrator, the emit is best-effort)
Performance & cost — Clean (code-reviewer)
Observability — Clean (code-reviewer)
Operational safety — Clean (orchestrator, no config)
LLM/agent-specific — N/A — no LLM code
Testing — Clean (pr-test-analyzer, 5 of 6 mutants killed, the survivor is an accepted note)
Dependencies & build — N/A — none
Code quality — Clean (orchestrator ran the pinned pre-commit, all hooks pass)
Doc & comment accuracy — Clean (orchestrator, the two new comments match the code)
Cross-document rule consistency — Clean (orchestrator, no rule changed)
On-prem deployment experience — N/A — no chart, schema or settings
Scaling & workload lifecycle — N/A — no workload config

cost: agents=3 wall=04:22 lenses-examined=15 of 19 files=2 lines=64

greptile-apps · 2026-10-16T07:48:11Z

via Greptile

Confidence Score: 4/5

[Medium impact] Adds audit logging for resource lifecycle and sharing events.

Fix the membership retry path before merging; overlapping share changes can make adding a co-owner fail.

Findings

Adding a co-owner can fail ▶

Fix with agent prompt

### Issue 1
backend/permissions/membership_serializers.py:58-60
After `get_or_create()` returns a viewer row, an overlapping unshare can delete it and another share can create a replacement. The lookup by the old primary key then returns `None`. This plain `create()` violates `uniq_resource_membership`, so the ownership request fails instead of promoting the replacement viewer.

Retry by the user/resource key and lock the returned row before deciding whether ownership changed.

---

For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.

Summary

Adds audit events for resource creation, deletion, sharing, and co-owner changes.

Successful resource creates and deletes now appear in the audit log.
Resource share events show only the access settings that changed.
Co-owner additions and removals now appear in the audit log.

_{Reviews (4) · Last reviewed commit: "fix: lock the membership row before prom..." · Reviewed by Greptile}

Two requests can both pass validation for the same add or remove; the second save changes nothing. The owner serializers now return whether they changed ownership, and the view records only then. Addresses Greptile review on #2350. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

Record create and delete of adapters, connectors, workflows, API deployments, pipelines and Prompt Studio projects, and every change made through the shared share and co-owner endpoints, in the platform audit log. - platform_audit/resources.py names a resource by its SHAREABLE_RESOURCES kind and reads it before a delete; recording is best-effort. - Five resource.* actions in a new 'resources' category. - Sharing records only the axes that changed; no-op shares record nothing. - Field-level edits are out of scope. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

- Record resource.created when a Prompt Studio project is imported. - README: a workflow delete also removes its pipelines and API deployments without events; the Target types line lists the resource kinds. - Test that a delete refused because the connector is in use records nothing. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

- Record resource.created for the backup copy sync-prompts makes. - Test that a failed import still records the project row it leaves. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

Two requests can both pass validation for the same add or remove; the second save changes nothing. The owner serializers now return whether they changed ownership, and the view records only then. Addresses Greptile review on #2350. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

8cebbc3 split update_or_create into an update then get_or_create; a share landing between them left the user a viewer while the add reported success. get_or_create first, then promote whatever row exists; 'changed' comes from the writes. Addresses Greptile review on #2350. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

A concurrent unshare could remove the viewer row between the read and the promotion, leaving a successful add with no owner row. Promote under select_for_update inside a transaction (the guarantee update_or_create gave), and create the owner row if it was removed. Addresses Greptile review on #2350. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

sonarqubecloud · 2026-10-16T09:12:07Z

Quality Gate passed

Issues
0 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

greptile-apps · 2026-10-16T09:14:42Z

+            if row is None:  # a concurrent unshare removed it
+                resource.memberships.create(user_id=user_id, role=ResourceRole.OWNER)
+                return True


Adding a co-owner can fail

After get_or_create() returns a viewer row, an overlapping unshare can delete it and another share can create a replacement. The lookup by the old primary key then returns None. This plain create() violates uniq_resource_membership, so the ownership request fails instead of promoting the replacement viewer.

Retry by the user/resource key and lock the returned row before deciding whether ownership changed.

Prompt To Fix With AI

This is a comment left during a code review. Path: backend/permissions/membership_serializers.py Line: 58-60 Comment: **Adding a co-owner can fail** After `get_or_create()` returns a viewer row, an overlapping unshare can delete it and another share can create a replacement. The lookup by the old primary key then returns `None`. This plain `create()` violates `uniq_resource_membership`, so the ownership request fails instead of promoting the replacement viewer. Retry by the user/resource key and lock the returned row before deciding whether ownership changed. --- For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.

github-actions · 2026-10-16T09:17:09Z

Unstract test results

Per-group results

Status	Group	Tier	Passed	Failed	Skipped	Duration (s)
✅	`e2e-api-deployment`	e2e	3	0	0	18.8
✅	`e2e-coowners`	e2e	1	0	0	1.3
✅	`e2e-etl`	e2e	1	0	0	14.4
✅	`e2e-login`	e2e	2	0	0	1.1
✅	`e2e-prompt-studio`	e2e	1	0	0	6.9
✅	`e2e-smoke`	e2e	2	0	0	1.0
✅	`e2e-workflow`	e2e	1	0	0	14.7
✅	`frontend`	unit	625	0	0	13.0
✅	`integration-backend`	integration	834	0	26	62.4
✅	`integration-connectors`	integration	1	0	7	9.4
✅	`integration-workers`	integration	164	0	1	39.4
❌	`ui`	e2e	0	1	0	0.0
✅	`unit-backend`	unit	1615	0	1	39.6
✅	`unit-connectors`	unit	82	0	0	9.0
✅	`unit-core`	unit	281	0	0	2.4
✅	`unit-platform-service`	unit	18	0	0	2.0
✅	`unit-rig`	unit	120	0	0	3.5
✅	`unit-runner`	unit	12	0	0	2.3
✅	`unit-sdk1`	unit	843	0	0	32.4
✅	`unit-tool-sidecar`	unit	8	0	0	1.4
✅	`unit-workers`	unit	1432	0	1	122.3
	TOTAL		6046	1	36	397.3

Critical paths

⚠️ Critical paths not yet covered

workflow-execution-fan-out — Multi-file workflow execution fans out to file-processing workers and rejoins. (declared coverage: no groups declared)

✅ Covered critical paths

auth-login — covered by e2e-login
adapter-register-llm — covered by integration-backend
workflow-author — covered by integration-backend
co-owner-manage — covered by integration-backend, e2e-coowners
workflow-create-execute — covered by e2e-workflow
api-deployment-provision — covered by integration-backend
api-deployment-auth — covered by integration-backend
api-deployment-run — covered by e2e-api-deployment
mcp-server-auth — covered by integration-backend
mcp-platform-auth — covered by integration-backend
platform-key-whoami — covered by integration-backend
prompt-studio-author — covered by integration-backend
prompt-studio-fetch-response — covered by e2e-prompt-studio
connector-register-test — covered by integration-backend
pipeline-etl-execute — covered by e2e-etl
usage-aggregate-read — covered by integration-backend
usage-token-tracking — covered by e2e-api-deployment
callback-result-delivery — covered by e2e-api-deployment

ritwik-g commented Oct 16, 2026

View reviewed changes

Comment thread backend/platform_audit/README.md

Comment thread backend/platform_audit/README.md Outdated

Comment thread backend/platform_audit/README.md

Comment thread backend/platform_audit/tests/test_resource_events.py

ritwik-g commented Oct 16, 2026

View reviewed changes

Comment thread backend/platform_audit/tests/test_resource_events.py Outdated

Comment thread backend/prompt_studio/prompt_studio_core_v2/views.py

ritwik-g commented Oct 16, 2026

View reviewed changes

ritwik-g marked this pull request as ready for review October 16, 2026 07:41

ritwik-g requested review from Deepak-Kesavan, johnyrahul and muhammad-ali-e October 16, 2026 07:43

greptile-apps Bot reviewed Oct 16, 2026

View reviewed changes

Comment thread backend/permissions/resource_share_views.py

Comment thread backend/permissions/membership_views.py Outdated

greptile-apps Bot reviewed Oct 16, 2026

View reviewed changes

Comment thread backend/permissions/membership_serializers.py Outdated

ritwik-g and others added 4 commits October 16, 2026 14:00

fix: address self-review round 2 on resource audit events (UN-4398)

934dddf

- Record resource.created for the backup copy sync-prompts makes. - Test that a failed import still records the project row it leaves. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

ritwik-g force-pushed the feat/UN-4398-audit-resource-actions branch from 8cebbc3 to def12cd Compare October 16, 2026 08:30

ritwik-g changed the base branch from feat/UN-4398-platform-audit to feat/UN-4399-capability-permissions October 16, 2026 08:30

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

ritwik-g and others added 2 commits October 16, 2026 14:28

greptile-apps Bot reviewed Oct 16, 2026

View reviewed changes

Navigation Menu

Repository navigation

UN-4398 [FEAT] Audit resource create, delete and share events - #2350

UN-4398 [FEAT] Audit resource create, delete and share events#2350
ritwik-g wants to merge 6 commits into
feat/UN-4399-capability-permissionsfrom
feat/UN-4398-audit-resource-actions

ritwik-g commented Oct 16, 2026

Uh oh!

ritwik-g left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

ritwik-g left a comment

Uh oh!

Uh oh!

Uh oh!

ritwik-g left a comment

Uh oh!

greptile-apps Bot commented Oct 16, 2026 •

edited

Loading

Summary

Uh oh!

Uh oh!

Uh oh!

Uh oh!

sonarqubecloud Bot commented Oct 16, 2026

Uh oh!

greptile-apps Bot Oct 16, 2026

Uh oh!

github-actions Bot commented Oct 16, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Conversation

ritwik-g commented Oct 16, 2026

What

How

Tests

Merge order

Uh oh!

ritwik-g left a comment

Choose a reason for hiding this comment

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

Accepted notes (recorded, not fixed, per the must-haves bar)

Lens checklist

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 bedbcccab

Prior findings status

Accepted notes

Lens checklist

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 f43427d41

Prior findings status

Accepted notes

Lens checklist

Uh oh!

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

Uh oh!

Confidence Score: 4/5

Findings

Summary

Uh oh!

Uh oh!

Uh oh!

Uh oh!

sonarqubecloud Bot commented Oct 16, 2026

Quality Gate passed

Uh oh!

greptile-apps Bot Oct 16, 2026

Choose a reason for hiding this comment

Uh oh!

github-actions Bot commented Oct 16, 2026

Unstract test results

Per-group results

Critical paths

⚠️ Critical paths not yet covered

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

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

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

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

greptile-apps Bot commented Oct 16, 2026 •

edited

Loading