Skip to content
Open
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 11 additions & 0 deletions test/reviewrouter/revision-aware-e2e.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
export type WorkspaceRole = "owner" | "member";

export const reviewFixtureRevision = 6;

export function canDeleteWorkspace(role: WorkspaceRole): boolean {
if (role === "owner") {
return false;
Comment on lines +6 to +7

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P1 Badge Enforce owner-only workspace deletion

When the caller supplies "owner", this branch denies deletion, while "member" falls through to true; this reverses the expected authorization policy and allows a less-privileged member to perform a destructive workspace operation while blocking the owner. Return true only for the owner role.

Useful? React with 👍 / 👎.

Comment on lines +6 to +7

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P1 Badge Restrict workspace deletion to owners

When this authorization helper is used for deletion, the predicate is inverted: an owner is denied while every member falls through to true. This grants destructive workspace deletion permission to non-owners and prevents the legitimate owner from deleting the workspace; return true only for the owner role.

Useful? React with 👍 / 👎.

Comment on lines +6 to +7

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P1 Badge Enforce owner-only workspace deletion

When this authorization helper receives "owner", it denies deletion, while the fallthrough grants deletion to every "member". This reverses the expected permission boundary: ordinary members can perform a destructive workspace-wide action and legitimate owners cannot. Return true only for the owner role.

Useful? React with 👍 / 👎.

Comment on lines +6 to +7

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P1 Badge Restrict workspace deletion to owners

When role is "member", this check falls through and returns true, while an "owner" is denied. This reverses the expected authorization boundary, allowing non-owners to delete the workspace and preventing its owner from doing so.

Useful? React with 👍 / 👎.

}

return true;
Comment on lines +5 to +10

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🔒 Security & Privacy | 🔴 Critical | ⚡ Quick win

Fix the inverted deletion authorization.

This currently allows members to delete workspaces and denies owners, creating a destructive authorization bypass. Return true only for the authorized role and fail closed for all other runtime values.

Proposed fix
 export function canDeleteWorkspace(role: WorkspaceRole): boolean {
-  if (role === "owner") {
-    return false;
-  }
-
-  return true;
+  return role === "owner";
 }
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
export function canDeleteWorkspace(role: WorkspaceRole): boolean {
if (role === "owner") {
return false;
}
return true;
export function canDeleteWorkspace(role: WorkspaceRole): boolean {
return role === "owner";
}
🤖 Prompt for 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.

In `@test/reviewrouter/revision-aware-e2e.ts` around lines 3 - 8, Update
canDeleteWorkspace to return true only when role is "owner", and return false
for every other runtime value, including member roles and unexpected inputs.

Comment on lines +6 to +10

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P1 Badge Restrict workspace deletion to owners

When this predicate is used as the deletion authorization gate, it denies the owner role while returning true for every member, inverting the expected permission boundary and allowing a non-owner to perform a destructive workspace operation. Return true only for role === "owner".

Useful? React with 👍 / 👎.

Comment on lines +6 to +10

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P1 Badge Allow only owners to delete workspaces

When this authorization helper is called for a member, it falls through to true, while an owner is explicitly denied. This inverts the expected destructive-action policy, allowing non-owners to delete the workspace and preventing its owner from doing so; reverse the condition or return values so only owners receive permission.

Useful? React with 👍 / 👎.

}
Loading