/approve-prs, /merge-prs 엔드포인트에 인증이 전혀 없고, 레포 제한도 빠져있음
Chưa có ai nhận issue này.
- Ngôn ngữ chính
- JavaScript
- Star
- 2
- Fork
- 2
- Chỉ số merge pull request
- Không có pull request nào được merge trong 30 ngày
Mô tả
배경
엔드포인트마다 인증 방식이 제각각입니다. /webhooks는 서명으로, /internal/*는 비밀 헤더로 확인합니다. 그런데 /check-weeks·/approve-prs·/merge-prs는 아무것도 확인하지 않습니다.
방어라고 할 만한 건 validateOrganization 하나뿐입니다. 요청 바디의 repo_owner가 "DaleStudy"인지만 봅니다. 그런데 이 값을 안 보내면 기본값이 "DaleStudy"라서 아무나 호출해도 통과합니다. 사실상 없는 것과 같습니다.
문제
/approve-prs·/merge-prs가 쓰는 parsePrActionPayload에는 repo_name을 leetcode-study로 묶는 체크가 없습니다. check-weeks.js에는 있습니다. CORS도 *라 브라우저에서 바로 호출할 수 있습니다.
결국 이 두 엔드포인트는 인증도 서명도 비밀값도 없이 바디의 repo_name만으로 동작합니다. 유일한 게이트인 조직 검증은 자기 신고라 있으나 마나입니다.
시도
cross-repo 쪽은 생각보다 week 필터가 어느 정도 막아줍니다. /approve-prs·/merge-prs는 week를 필수로 받고 각 PR의 Projects v2 "Week" 필드로 거릅니다. Week 필드가 없는 PR은 week: null이라 어떤 week를 넣어도 걸리지 않고 스킵됩니다(approved: 0).
그래서 leetcode-study 보드에 없는 레포는 이 두 엔드포인트로 승인하거나 병합할 수 없습니다. daleui.com의 실제 dependabot PR(DaleStudy/daleui.com#86)로 확인해봤는데 걸리지 않습니다. 임의 레포 무인증 승인은 Week 필드가 있는 레포에서만 됩니다.
물론 승인까지는 안 되더라도 App 토큰으로 PR 목록이나 프로젝트 필드를 읽는 호출은 무인증으로 그냥 나갑니다(App이 설치된 레포에 한합니다).
위험
문제는 leetcode-study 자체입니다. 여기는 Week 필드가 있으니 아무나 {"repo_name":"leetcode-study","week":"<현재 주차>"} 하나만 던지면 인증 없이 그 주차 PR을 통째로 승인하거나 auto-merge를 걸 수 있습니다. 승인과 병합은 되돌리기 힘든 동작이라 곧바로 사고로 이어집니다.
앞으로 이 보드(Week 필드)를 쓰는 레포가 늘어나면 그 레포들도 똑같이 노출됩니다.
제안
/approve-prs·/merge-prs(그리고/check-weeks)에도/internal/*처럼 요청 인증(비밀 헤더나 서명)을 붙입니다. week 필터는 믿으면 안 됩니다. 어쩌다 막힌 것이지 인증이 아닙니다.parsePrActionPayload에도check-weeks.js처럼repo_name === ALLOWED_REPO체크를 넣습니다. 허용 레포 목록으로 관리해도 됩니다.validateOrganization이 바디 자기 신고 대신 실제 인증된 컨텍스트를 보도록 고칩니다.
Hướng dẫn đóng góp
Bắt đầu từ đâu
- Đọc hết issue, rồi đọc hướng dẫn đóng góp của dự án.
- Bình luận trên issue rằng bạn sẽ nhận — tránh hai người làm cùng một việc.
- Fork repository và làm thay đổi trên một nhánh.
- Mở pull request có tham chiếu số hiệu của issue.
Hướng nghiên cứu
Bắt đầu với các handler của /approve-prs, /merge-prs và /check-weeks, sau đó kiểm tra parsePrActionPayload và cơ chế xác thực được /internal/* sử dụng. So sánh giới hạn repository trong check-weeks.js với đường dẫn payload của hành động PR. Được xem là hoàn tất khi các endpoint hành động yêu cầu xác thực request, giới hạn repository và xác thực quyền truy cập tổ chức từ ngữ cảnh đã xác thực thay vì dữ liệu do người dùng cung cấp.
Do mô hình lập chỉ mục viết ra từ nội dung của issue.
Đánh giá
- Công nghệ
- javascript
- Lĩnh vực
- api, authentication, security
- Loại issue
- Lỗi
- Độ khó
- 4/5
- Thời gian dự kiến
- 3-5 ngày
- Mức độ hoạt động
- Ít trao đổi
- Độ rõ ràng
- Khá rõ ràng
- Mức phù hợp với người mới
- 48/100