refactor(api): require permission key in create and update requests by AmanGIT07 · Pull Request #1885 · raystack/frontier

AmanGIT07 · 2026-08-14T09:36:20Z

Summary

CreatePermission and UpdatePermission accept the permission identity only via the key field (service.resource.verb). The deprecated namespace/name request body fields are no longer read; requests sending only those fields now receive InvalidArgument. Part of #1782.

Changes

internal/api/v1beta1connect/permission.go: remove the namespace/name fallback; reject a missing or malformed key in both CreatePermission and UpdatePermission
cmd/seed/permissions.json + cmd/seed.go: seed data sends key
test/e2e/regression/service_registration_test.go: request bodies send key
internal/api/v1beta1connect/permission_test.go: request bodies send key; add missing-key and malformed-key cases for both handlers

Test Plan

go test ./internal/api/v1beta1connect/ passes
make lint passes (0 issues)
Full e2e regression suite ran successfully

🤖 Generated with Claude Code

CreatePermission and UpdatePermission read the permission identity only from the key field and return InvalidArgument when it is missing or malformed. The deprecated namespace/name request fields are no longer read. The seed command and e2e tests send keys. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

vercel · 2026-08-14T09:36:27Z

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

Project	Deployment	Actions	Updated (UTC)
frontier	Ready	Preview	Aug 14, 2026 10:42am

coderabbitai · 2026-08-14T09:36:49Z

📝 Walkthrough

Summary by CodeRabbit

New Features
- Permission identifiers now use a single key format for creation, updates, and bootstrap configuration.
Bug Fixes
- Invalid or incomplete permission keys now return clear validation errors instead of being accepted through legacy fields.
- Permission names and namespaces are now validated consistently.
- Permission metadata formatting has been standardized.
Tests
- Added coverage for missing, malformed, and invalid permission keys.
- Updated regression scenarios to validate the consolidated permission format.

Walkthrough

The permission model now uses a consolidated Key field. Seed data, custom permission reporting, API validation, unit tests, and regression tests use fully qualified permission keys.

Changes

Permission key migration

Layer / File(s)	Summary
Seed data and custom permission reporting `cmd/seed.go`, `cmd/seed/permissions.json`	Seed permissions use combined keys. Custom permission reporting parses namespace and name from each key.
API key validation `internal/api/v1beta1connect/errors.go`, `internal/api/v1beta1connect/permission.go`	`CreatePermission` and `UpdatePermission` validate parsed key components and return `InvalidArgument` for incomplete or invalid keys.
Request and regression coverage `internal/api/v1beta1connect/permission_test.go`, `test/e2e/regression/service_registration_test.go`	Tests submit consolidated keys and cover validation, successful operations, and deletion scenarios.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Merge Risk: ⚪ Minimal · up to c28de

The PR enforces key-only permission requests and updates the relevant callers and tests; no actionable merge-blocking risk remains.

Possibly related PRs

raystack/frontier#1767: Both PRs modify permission handling in permission.go.
raystack/frontier#1797: Both PRs modify permission namespace validation and schema handling.
raystack/frontier#1886: Both PRs consolidate permission-related API inputs.

Suggested reviewers: rohilsurana

🚥 Pre-merge checks | ✅ 2

✅ Passed checks (2 passed)

Check name	Status	Explanation
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.

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

Actionable comments posted: 1

🧹 Nitpick comments (1)

internal/api/v1beta1connect/permission_test.go (1)

84-102: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick win

Prove that deprecated fields are ignored.

The missing-key cases leave Name and Namespace empty. A future fallback to populated deprecated fields could pass these cases. Add CreatePermission and UpdatePermission cases with Key empty, Name and Namespace populated, and expect CodeInvalidArgument.

This follows the PR objective that deprecated request fields are no longer read.

Also applies to: 268-286

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: 9a841b60-53d3-4101-b77a-15ff9171c906

📥 Commits

Reviewing files that changed from the base of the PR and between 7c10c55 and 6b786f6.

📒 Files selected for processing (5)

cmd/seed.go
cmd/seed/permissions.json
internal/api/v1beta1connect/permission.go
internal/api/v1beta1connect/permission_test.go
test/e2e/regression/service_registration_test.go

coveralls · 2026-08-14T09:42:59Z

Coverage Report for CI Build 31793240068

Coverage increased (+0.003%) to 48.701%

Details

Coverage increased (+0.003%) from the base build.
Patch coverage: 4 uncovered changes across 2 files (9 of 13 lines covered, 69.23%).
1 coverage regression across 1 file.

Uncovered Changes

File	Changed	Covered	%
cmd/seed.go	3	0	0.0%
internal/api/v1beta1connect/permission.go	10	9	90.0%

Coverage Regressions

1 previously-covered line in 1 file lost coverage.

File	Lines Losing Coverage	Coverage
internal/api/v1beta1connect/permission.go	1	59.6%

Coverage Stats


Relevant Lines:	40083
Covered Lines:	19521
Line Coverage:	48.7%
Coverage Strength:	15.65 hits per line

💛 - Coveralls

Missing or malformed keys now fail with a format-hint error instead of a generic bad request. Create validates the namespace form and Update applies the same name and namespace checks as Create. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

coderabbitai

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)

internal/api/v1beta1connect/permission_test.go (1)
275-315: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win

Add an invalid-namespace case for UpdatePermission.

internal/api/v1beta1connect/permission.go Line 120 has a separate namespace-validation branch. TestHandler_UpdatePermission covers missing keys, malformed keys, and invalid permission names, but it does not cover a three-part key with an invalid namespace. Add an app..get case and expect connect.CodeInvalidArgument with ErrPermissionKeyNotation.

Based on the changed handler branch and the supplied test cases, this is the only new validation path without a direct regression case.
Proposed test case
+		{
+			name:  "should return bad request error if key namespace is invalid",
+			setup: func(as *mocks.PermissionService) {},
+			request: connect.NewRequest(&frontierv1beta1.UpdatePermissionRequest{
+				Id: testPermissions[testPermissionIdx].ID,
+				Body: &frontierv1beta1.PermissionRequestBody{
+					Key: "app..get",
+				},
+			}),
+			want:    nil,
+			wantErr: connect.NewError(connect.CodeInvalidArgument, ErrPermissionKeyNotation),
+		},

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: d7469106-ebfb-4b52-94ba-265fdc69e424

📥 Commits

Reviewing files that changed from the base of the PR and between 6b786f6 and c28de61.

📒 Files selected for processing (3)

internal/api/v1beta1connect/errors.go
internal/api/v1beta1connect/permission.go
internal/api/v1beta1connect/permission_test.go

vercel Bot deployed to Preview August 14, 2026 09:36 View deployment

coderabbitai Bot reviewed Aug 14, 2026

View reviewed changes

Comment thread internal/api/v1beta1connect/permission.go Outdated

vercel Bot deployed to Preview August 14, 2026 10:42 View deployment

coderabbitai Bot reviewed Aug 14, 2026

View reviewed changes

Navigation Menu

Uh oh!

refactor(api): require permission key in create and update requests - #1885

refactor(api): require permission key in create and update requests#1885
AmanGIT07 wants to merge 2 commits into
mainfrom
refactor/drop-deprecated-request-fallbacks

AmanGIT07 commented Aug 14, 2026

Uh oh!

vercel Bot commented Aug 14, 2026 •

edited

Loading

Uh oh!

coderabbitai Bot commented Aug 14, 2026 •

edited

Loading

Summary by CodeRabbit

Walkthrough

Changes

Uh oh!

coderabbitai Bot left a comment

Uh oh!

Uh oh!

coveralls commented Aug 14, 2026 •

edited

Loading

Uh oh!

coderabbitai Bot left a comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Uh oh!

Conversation

AmanGIT07 commented Aug 14, 2026

Summary

Changes

Test Plan

Uh oh!

vercel Bot commented Aug 14, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

coderabbitai Bot commented Aug 14, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary by CodeRabbit

Walkthrough

Changes

Uh oh!

coderabbitai Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

coveralls commented Aug 14, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Coverage Report for CI Build 31793240068

Coverage increased (+0.003%) to 48.701%

Details

Uncovered Changes

Coverage Regressions

Coverage Stats

💛 - Coveralls

Uh oh!

coderabbitai Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

vercel Bot commented Aug 14, 2026 •

edited

Loading

coderabbitai Bot commented Aug 14, 2026 •

edited

Loading

coveralls commented Aug 14, 2026 •

edited

Loading