git-pr / fix(cli): access control for removing patchsets #6

accepted · opened on 2024-07-19T16:47:38Z by erock
Help
# add changes to patch request
git format-patch main --stdout | ssh pr.pico.sh pr add 6
# add review to patch request
git format-patch main --stdout | ssh pr.pico.sh pr add --review 6
# remove patchset
ssh pr.pico.sh ps rm ps-x
# checkout all patches
ssh pr.pico.sh pr print 6 | git am -3
# print a diff between the last two patches in a patch request
ssh pr.pico.sh pr diff 6
# accept PR
ssh pr.pico.sh pr accept 6
# close PR
ssh pr.pico.sh pr close 6

Logs

erock created pr with ps-8 on 2024-07-19T16:47:38Z
erock changed status on 2024-07-19T17:47:15Z {"status":"accepted"}

Patchsets

Diff ↕
I also fixed some other access control issues for changing PR status.
 cli.go | 36 ++++++++++++++++++++++++++++++------
 pr.go  | 11 +++++++++++
 2 files changed, 41 insertions(+), 6 deletions(-)
  1From ea484661df8b36a3130ed9ebf4b1f0b05e5ed0a4 Mon Sep 17 00:00:00 2001
  2From: Eric Bower <me@erock.io>
  3Date: Fri, 19 Jul 2024 12:20:18 -0400
  4Subject: [PATCH] fix(cli): access control for removing patchsets
  5
  6I also fixed some other access control issues for changing PR status.
  7---
  8 cli.go | 36 ++++++++++++++++++++++++++++++------
  9 pr.go  | 11 +++++++++++
 10 2 files changed, 41 insertions(+), 6 deletions(-)
 11
 12diff --git a/cli.go b/cli.go
 13index fcd2543..16c696d 100644
 14--- a/cli.go
 15+++ b/cli.go
 16@@ -213,7 +213,30 @@ Here's how it works:
 17 							if err != nil {
 18 								return err
 19 							}
 20-							return pr.DeletePatchsetByID(patchsetID)
 21+
 22+							patchset, err := pr.GetPatchsetByID(patchsetID)
 23+							if err != nil {
 24+								return err
 25+							}
 26+
 27+							user, err := pr.GetUserByID(patchset.UserID)
 28+							if err != nil {
 29+								return err
 30+							}
 31+
 32+							pk := sesh.PublicKey()
 33+							isAdmin := be.IsAdmin(pk)
 34+							isContrib := pubkey == user.Pubkey
 35+							if !isAdmin && !isContrib {
 36+								return fmt.Errorf("you are not authorized to delete a patchset")
 37+							}
 38+
 39+							err = pr.DeletePatchsetByID(patchsetID)
 40+							if err != nil {
 41+								return err
 42+							}
 43+							wish.Printf(sesh, "successfully removed patchset: %d\n", patchsetID)
 44+							return nil
 45 						},
 46 					},
 47 				},
 48@@ -597,17 +620,18 @@ Here's how it works:
 49 								return err
 50 							}
 51 
 52-							user, err := pr.UpsertUser(pubkey, userName)
 53+							patchReq, err := pr.GetPatchRequestByID(prID)
 54 							if err != nil {
 55 								return err
 56 							}
 57 
 58-							patchReq, err := pr.GetPatchRequestByID(prID)
 59+							user, err := pr.GetUserByID(patchReq.UserID)
 60 							if err != nil {
 61 								return err
 62 							}
 63+
 64 							pk := sesh.PublicKey()
 65-							isContrib := be.Pubkey(pk) == user.Pubkey
 66+							isContrib := pubkey == user.Pubkey
 67 							isAdmin := be.IsAdmin(pk)
 68 							if !isAdmin && !isContrib {
 69 								return fmt.Errorf("you are not authorized to change PR status")
 70@@ -645,13 +669,13 @@ Here's how it works:
 71 								return err
 72 							}
 73 
 74-							user, err := pr.UpsertUser(pubkey, userName)
 75+							user, err := pr.GetUserByID(patchReq.UserID)
 76 							if err != nil {
 77 								return err
 78 							}
 79 
 80 							pk := sesh.PublicKey()
 81-							isContrib := be.Pubkey(pk) == user.Pubkey
 82+							isContrib := pubkey == user.Pubkey
 83 							isAdmin := be.IsAdmin(pk)
 84 							if !isAdmin && !isContrib {
 85 								return fmt.Errorf("you are not authorized to change PR status")
 86diff --git a/pr.go b/pr.go
 87index 95c0b11..e4b3389 100644
 88--- a/pr.go
 89+++ b/pr.go
 90@@ -34,6 +34,7 @@ type GitPatchRequest interface {
 91 	GetPatchRequests() ([]*PatchRequest, error)
 92 	GetPatchRequestsByRepoID(repoID string) ([]*PatchRequest, error)
 93 	GetPatchsetsByPrID(prID int64) ([]*Patchset, error)
 94+	GetPatchsetByID(patchsetID int64) (*Patchset, error)
 95 	GetLatestPatchsetByPrID(prID int64) (*Patchset, error)
 96 	GetPatchesByPatchsetID(prID int64) ([]*Patch, error)
 97 	UpdatePatchRequestStatus(prID, userID int64, status string) error
 98@@ -234,6 +235,16 @@ func (pr PrCmd) GetPatchsetsByPrID(prID int64) ([]*Patchset, error) {
 99 	return patchsets, nil
100 }
101 
102+func (pr PrCmd) GetPatchsetByID(patchsetID int64) (*Patchset, error) {
103+	var patchset Patchset
104+	err := pr.Backend.DB.Get(
105+		&patchset,
106+		"SELECT * FROM patchsets WHERE id=?",
107+		patchsetID,
108+	)
109+	return &patchset, err
110+}
111+
112 func (pr PrCmd) GetLatestPatchsetByPrID(prID int64) (*Patchset, error) {
113 	patchsets, err := pr.GetPatchsetsByPrID(prID)
114 	if err != nil {
115-- 
1162.45.2
117
ps-8 by erock on 2024-07-19T16:47:38Z

fix(cli): access control for removing patchsets

Eric Bower <me@erock.io> 2024-07-19T16:20:18Z
I also fixed some other access control issues for changing PR status.
 cli.go | 36 ++++++++++++++++++++++++++++++------
 pr.go  | 11 +++++++++++
 2 files changed, 41 insertions(+), 6 deletions(-)
  1From ea484661df8b36a3130ed9ebf4b1f0b05e5ed0a4 Mon Sep 17 00:00:00 2001
  2From: Eric Bower <me@erock.io>
  3Date: Fri, 19 Jul 2024 12:20:18 -0400
  4Subject: [PATCH] fix(cli): access control for removing patchsets
  5
  6I also fixed some other access control issues for changing PR status.
  7---
  8 cli.go | 36 ++++++++++++++++++++++++++++++------
  9 pr.go  | 11 +++++++++++
 10 2 files changed, 41 insertions(+), 6 deletions(-)
 11
 12diff --git a/cli.go b/cli.go
 13index fcd2543..16c696d 100644
 14--- a/cli.go
 15+++ b/cli.go
 16@@ -213,7 +213,30 @@ Here's how it works:
 17 							if err != nil {
 18 								return err
 19 							}
 20-							return pr.DeletePatchsetByID(patchsetID)
 21+
 22+							patchset, err := pr.GetPatchsetByID(patchsetID)
 23+							if err != nil {
 24+								return err
 25+							}
 26+
 27+							user, err := pr.GetUserByID(patchset.UserID)
 28+							if err != nil {
 29+								return err
 30+							}
 31+
 32+							pk := sesh.PublicKey()
 33+							isAdmin := be.IsAdmin(pk)
 34+							isContrib := pubkey == user.Pubkey
 35+							if !isAdmin && !isContrib {
 36+								return fmt.Errorf("you are not authorized to delete a patchset")
 37+							}
 38+
 39+							err = pr.DeletePatchsetByID(patchsetID)
 40+							if err != nil {
 41+								return err
 42+							}
 43+							wish.Printf(sesh, "successfully removed patchset: %d\n", patchsetID)
 44+							return nil
 45 						},
 46 					},
 47 				},
 48@@ -597,17 +620,18 @@ Here's how it works:
 49 								return err
 50 							}
 51 
 52-							user, err := pr.UpsertUser(pubkey, userName)
 53+							patchReq, err := pr.GetPatchRequestByID(prID)
 54 							if err != nil {
 55 								return err
 56 							}
 57 
 58-							patchReq, err := pr.GetPatchRequestByID(prID)
 59+							user, err := pr.GetUserByID(patchReq.UserID)
 60 							if err != nil {
 61 								return err
 62 							}
 63+
 64 							pk := sesh.PublicKey()
 65-							isContrib := be.Pubkey(pk) == user.Pubkey
 66+							isContrib := pubkey == user.Pubkey
 67 							isAdmin := be.IsAdmin(pk)
 68 							if !isAdmin && !isContrib {
 69 								return fmt.Errorf("you are not authorized to change PR status")
 70@@ -645,13 +669,13 @@ Here's how it works:
 71 								return err
 72 							}
 73 
 74-							user, err := pr.UpsertUser(pubkey, userName)
 75+							user, err := pr.GetUserByID(patchReq.UserID)
 76 							if err != nil {
 77 								return err
 78 							}
 79 
 80 							pk := sesh.PublicKey()
 81-							isContrib := be.Pubkey(pk) == user.Pubkey
 82+							isContrib := pubkey == user.Pubkey
 83 							isAdmin := be.IsAdmin(pk)
 84 							if !isAdmin && !isContrib {
 85 								return fmt.Errorf("you are not authorized to change PR status")
 86diff --git a/pr.go b/pr.go
 87index 95c0b11..e4b3389 100644
 88--- a/pr.go
 89+++ b/pr.go
 90@@ -34,6 +34,7 @@ type GitPatchRequest interface {
 91 	GetPatchRequests() ([]*PatchRequest, error)
 92 	GetPatchRequestsByRepoID(repoID string) ([]*PatchRequest, error)
 93 	GetPatchsetsByPrID(prID int64) ([]*Patchset, error)
 94+	GetPatchsetByID(patchsetID int64) (*Patchset, error)
 95 	GetLatestPatchsetByPrID(prID int64) (*Patchset, error)
 96 	GetPatchesByPatchsetID(prID int64) ([]*Patch, error)
 97 	UpdatePatchRequestStatus(prID, userID int64, status string) error
 98@@ -234,6 +235,16 @@ func (pr PrCmd) GetPatchsetsByPrID(prID int64) ([]*Patchset, error) {
 99 	return patchsets, nil
100 }
101 
102+func (pr PrCmd) GetPatchsetByID(patchsetID int64) (*Patchset, error) {
103+	var patchset Patchset
104+	err := pr.Backend.DB.Get(
105+		&patchset,
106+		"SELECT * FROM patchsets WHERE id=?",
107+		patchsetID,
108+	)
109+	return &patchset, err
110+}
111+
112 func (pr PrCmd) GetLatestPatchsetByPrID(prID int64) (*Patchset, error) {
113 	patchsets, err := pr.GetPatchsetsByPrID(prID)
114 	if err != nil {
115-- 
1162.45.2
117