Skip to content

/approve-prs, /merge-prs 엔드포인트에 인증이 전혀 없고, 레포 제한도 빠져있음 #46

Description

@SimYunSup

배경

엔드포인트마다 인증 방식이 제각각입니다. /webhooks는 서명으로, /internal/*는 비밀 헤더로 확인합니다. 그런데 /check-weeks·/approve-prs·/merge-prs는 아무것도 확인하지 않습니다.

방어라고 할 만한 건 validateOrganization 하나뿐입니다. 요청 바디의 repo_owner"DaleStudy"인지만 봅니다. 그런데 이 값을 안 보내면 기본값이 "DaleStudy"라서 아무나 호출해도 통과합니다. 사실상 없는 것과 같습니다.

문제

/approve-prs·/merge-prs가 쓰는 parsePrActionPayload에는 repo_nameleetcode-study로 묶는 체크가 없습니다. check-weeks.js에는 있습니다. CORS도 *라 브라우저에서 바로 호출할 수 있습니다.

결국 이 두 엔드포인트는 인증도 서명도 비밀값도 없이 바디의 repo_name만으로 동작합니다. 유일한 게이트인 조직 검증은 자기 신고라 있으나 마나입니다.

시도

cross-repo 쪽은 생각보다 week 필터가 어느 정도 막아줍니다. /approve-prs·/merge-prsweek를 필수로 받고 각 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이 바디 자기 신고 대신 실제 인증된 컨텍스트를 보도록 고칩니다.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Fields

    No fields configured for issues without a type.

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions