Skip to content

feat: update recording, transcript endpoint and add tests #16

Open
tomerqodo wants to merge 2 commits into
sentry_full_base_feat_update_recording_transcript_endpoint_and_add_tests__pr16from
sentry_full_head_feat_update_recording_transcript_endpoint_and_add_tests__pr16
Open

feat: update recording, transcript endpoint and add tests #16
tomerqodo wants to merge 2 commits into
sentry_full_base_feat_update_recording_transcript_endpoint_and_add_tests__pr16from
sentry_full_head_feat_update_recording_transcript_endpoint_and_add_tests__pr16

Conversation

@tomerqodo

Copy link
Copy Markdown

Benchmark PR from agentic-review-benchmarks#16

@UseGuards(BookingUidGuard)
// @UseGuards(ApiAuthGuard, BookingUidGuard, BookingPbacGuard)
@Pbac(["booking.readRecordings"])
@Permissions([BOOKING_WRITE])

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Bug: The getBookingRecordings GET endpoint incorrectly requires BOOKING_WRITE permission instead of BOOKING_READ, which is inconsistent with its read-only function and other similar endpoints.
Severity: MEDIUM

Suggested Fix

Change the @Permissions decorator for the getBookingRecordings endpoint from [BOOKING_WRITE] to [BOOKING_READ]. This aligns the permission with the endpoint's read-only functionality and makes it consistent with other GET endpoints in the controller.

Prompt for AI Agent
Review the code at the location below. A potential bug has been identified by an AI
agent.
Verify if this is a real issue. If it is, propose a fix; if not, explain why it's not
valid.

Location: apps/api/v2/src/ee/bookings/2024-08-13/controllers/bookings.controller.ts#L216

Potential issue: The `getBookingRecordings` endpoint, a GET request for fetching data,
incorrectly requires `BOOKING_WRITE` permission. This contradicts its read-only nature,
the associated `@Pbac(["booking.readRecordings"])` decorator which implies a read
operation, and other similar GET endpoints in the same controller that use
`BOOKING_READ`. This will cause permission failures for users who have read access but
not write access to bookings, preventing them from viewing recordings they should be
able to access, violating the principle of least privilege.

Did we get this right? 👍 / 👎 to inform future reviews.

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