Skip to content

fix(requests): isolate playground requests by user - #2206

Open
brynne8 wants to merge 3 commits into
looplj:unstablefrom
brynne8:fix/playground-request-user-isolation
Open

fix(requests): isolate playground requests by user#2206
brynne8 wants to merge 3 commits into
looplj:unstablefrom
brynne8:fix/playground-request-user-isolation

Conversation

@brynne8

@brynne8 brynne8 commented Aug 12, 2026

Copy link
Copy Markdown
Contributor

Summary

  • persist the authenticated creator for Playground requests without exposing the field through GraphQL
  • restrict project members to their own Playground requests and usage logs
  • retain project-wide audit visibility for project owners and system-scoped request readers
  • hide historical unowned Playground requests from regular members

Tests

  • go test ./internal/scopes -count=1
  • go test ./internal/server/biz -run 'TestRequestService(CreateRequestAssignsPlaygroundUser|CreateRequestExecutionPersistsReasoningEffort)' -count=1

Summary by CodeRabbit

  • New Features

    • Playground requests are now associated with their authenticated creator.
    • Regular members see only their own Playground requests, while shared API requests remain accessible.
    • Project owners and authorized system users retain access to all project requests.
    • Requests can be filtered and sorted by creator.
  • Tests

    • Added coverage for request ownership, visibility isolation, and unauthorized access behavior.

Playground requests now retain their authenticated creator and project members can only query their own records. Project owners and users with system-level request access retain project audit visibility.

- add an internal request creator field and project-user index
- apply creator filtering to requests and usage logs
- cover persistence and privacy isolation with regression tests
@coderabbitai

coderabbitai Bot commented Aug 12, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: f461698a-2dff-428c-9f6d-87d11562a451

📥 Commits

Reviewing files that changed from the base of the PR and between 2384ba1 and ec31e54.

📒 Files selected for processing (2)
  • internal/ent/internal/schema.go
  • internal/ent/migrate/schema.go
🚧 Files skipped from review as they are similar to previous changes (1)
  • internal/ent/migrate/schema.go

📝 Walkthrough

Walkthrough

The Request schema now stores an optional immutable creator user ID. Playground requests assign this ID from the authenticated user. Project request scopes use it to isolate Playground requests while preserving owner, system-scope, and permitted API request access.

Changes

Request creator isolation

Layer / File(s) Summary
Request user ID schema and Ent support
internal/ent/schema/request.go, internal/ent/migrate/schema.go, internal/ent/request/..., internal/ent/request.go, internal/ent/mutation.go, internal/ent/request_create.go, internal/ent/request_update.go, internal/ent/entql.go, internal/ent/runtime/runtime.go
The Request model and generated Ent code support nullable user_id storage, predicates, ordering, mutation operations, scanning, updates, and a composite index.
Creator assignment during request creation
internal/server/biz/request.go, internal/server/biz/request_user_isolation_test.go
Playground requests store the authenticated user ID. API requests do not store this association.
Scoped request visibility
internal/scopes/rule_user_project_scope_requests.go, internal/scopes/rule_user_project_scope_requests_test.go
Members see their own Playground requests and permitted API requests. Project owners and system-scope users see all project requests. Tests cover these access paths.

Estimated code review effort: 4 (Complex) | ~45 minutes

Mergeability Score: ⚪ Minimal · up to ec31e

The change isolates Playground requests by user while preserving owner and system audit access. No actionable merge-blocking risk remains beyond normal checks and review.

Sequence Diagram(s)

sequenceDiagram
  participant AuthenticatedUser
  participant RequestService
  participant EntRequest
  participant ProjectScopeRule
  AuthenticatedUser->>RequestService: Create Playground request
  RequestService->>EntRequest: Persist user_id
  AuthenticatedUser->>ProjectScopeRule: Read project requests
  ProjectScopeRule->>EntRequest: Apply creator and API-key filters
  EntRequest-->>ProjectScopeRule: Return matching requests
  ProjectScopeRule-->>AuthenticatedUser: Return authorized results
Loading

Possibly related PRs

  • looplj/axonhub#2095: Both PRs add generated Ent support for a nullable integer field, but they target different entities and features.
🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and concisely describes the primary change: isolating Playground requests by user.
Docstring Coverage ✅ Passed Docstring coverage is 81.82% which is sufficient. The required threshold is 80.00%.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@internal/server/biz/request.go`:
- Around line 227-231: Update the Playground branch around contexts.GetUser to
return an authentication error when no authenticated user is present, and only
create the request after assigning the authenticated user’s ID via
mut.SetUserID. Add a regression test covering a Playground request without a
user and assert the authentication error.
🪄 Autofix

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: cd6a27f5-2727-48a5-b537-2e313333bc81

📥 Commits

Reviewing files that changed from the base of the PR and between c8de8cf and 2a9c749.

📒 Files selected for processing (15)
  • internal/ent/entql.go
  • internal/ent/internal/schema.go
  • internal/ent/migrate/schema.go
  • internal/ent/mutation.go
  • internal/ent/request.go
  • internal/ent/request/request.go
  • internal/ent/request/where.go
  • internal/ent/request_create.go
  • internal/ent/request_update.go
  • internal/ent/runtime/runtime.go
  • internal/ent/schema/request.go
  • internal/scopes/rule_user_project_scope_requests.go
  • internal/scopes/rule_user_project_scope_requests_test.go
  • internal/server/biz/request.go
  • internal/server/biz/request_user_isolation_test.go

Comment thread internal/server/biz/request.go
Add the missing docstring required by repository documentation coverage checks.
…quest-user-isolation

# Conflicts:
#	internal/ent/internal/schema.go
@brynne8

brynne8 commented Aug 18, 2026

Copy link
Copy Markdown
Contributor Author

@looplj 是不是可以合并这个了?准备合并的话我再把冲突改了,不然还需要一直改。

这个PR处理的问题就是playground所有人都可见的问题。应该自己只能看见自己的游乐场才对。

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant