fix(gmail): harden labels delete id resolution
This commit is contained in:
Peter Steinberger
parent
48a2ad6ed5
commit
49eb68b412
@ -19,6 +19,7 @@
|
||||
|
||||
- Formatting: `make fmt` (`goimports` local prefix `github.com/steipete/gogcli` + `gofumpt`).
|
||||
- Output: keep stdout parseable (`--json` / `--plain`); send human hints/progress to stderr.
|
||||
- Gmail labels: treat label IDs as case-sensitive opaque tokens; only case-fold label names for name lookup.
|
||||
|
||||
## Testing Guidelines
|
||||
|
||||
|
||||
@ -9,6 +9,7 @@
|
||||
- Drive: share files with an entire Workspace domain via `drive share --to domain`. (#192) — thanks @Danielkweber.
|
||||
- Drive: add upload conversion flags `--convert` (auto) and `--convert-to` (`doc|sheet|slides`). (#240) — thanks @Danielkweber.
|
||||
- Docs: add `docs create --file` to import Markdown into Google Docs with inline image support and hardened temp-file cleanup. (#244) — thanks @chrismdp.
|
||||
- Gmail: add `gmail labels delete <labelIdOrName>` with confirm + system-label guardrails and case-sensitive ID handling. (#231) — thanks @Helmi.
|
||||
|
||||
### Fixed
|
||||
|
||||
|
||||
@ -227,6 +227,21 @@ func fetchLabelNameToID(svc *gmail.Service) (map[string]string, error) {
|
||||
return m, nil
|
||||
}
|
||||
|
||||
func fetchLabelNameOnlyToID(svc *gmail.Service) (map[string]string, error) {
|
||||
resp, err := svc.Users.Labels.List("me").Do()
|
||||
if err != nil {
|
||||
return nil, err
|
||||
}
|
||||
m := make(map[string]string, len(resp.Labels))
|
||||
for _, l := range resp.Labels {
|
||||
if l.Id == "" || l.Name == "" {
|
||||
continue
|
||||
}
|
||||
m[strings.ToLower(l.Name)] = l.Id
|
||||
}
|
||||
return m, nil
|
||||
}
|
||||
|
||||
type GmailLabelsDeleteCmd struct {
|
||||
Label string `arg:"" name:"labelIdOrName" help:"Label ID or name"`
|
||||
}
|
||||
@ -248,11 +263,14 @@ func (c *GmailLabelsDeleteCmd) Run(ctx context.Context, flags *RootFlags) error
|
||||
return usage("empty label")
|
||||
}
|
||||
|
||||
// For destructive operations, try exact ID match first before name lookup
|
||||
// For destructive operations, try exact ID match first before name lookup.
|
||||
label, err := svc.Users.Labels.Get("me", raw).Context(ctx).Do()
|
||||
if err != nil {
|
||||
// Not a valid ID, try resolving as name
|
||||
idMap, mapErr := fetchLabelNameToID(svc)
|
||||
if !isNotFoundAPIError(err) {
|
||||
return err
|
||||
}
|
||||
// Exact ID not found; resolve by label name only.
|
||||
idMap, mapErr := fetchLabelNameOnlyToID(svc)
|
||||
if mapErr != nil {
|
||||
return mapErr
|
||||
}
|
||||
|
||||
304
internal/cmd/gmail_labels_delete_cmd_test.go
Normal file
304
internal/cmd/gmail_labels_delete_cmd_test.go
Normal file
@ -0,0 +1,304 @@
|
||||
package cmd
|
||||
|
||||
import (
|
||||
"context"
|
||||
"encoding/json"
|
||||
"io"
|
||||
"net/http"
|
||||
"net/http/httptest"
|
||||
"strings"
|
||||
"testing"
|
||||
|
||||
"google.golang.org/api/gmail/v1"
|
||||
"google.golang.org/api/option"
|
||||
|
||||
"github.com/steipete/gogcli/internal/outfmt"
|
||||
"github.com/steipete/gogcli/internal/ui"
|
||||
)
|
||||
|
||||
func newLabelsDeleteService(t *testing.T, handler http.HandlerFunc) {
|
||||
t.Helper()
|
||||
|
||||
srv := httptest.NewServer(handler)
|
||||
t.Cleanup(srv.Close)
|
||||
|
||||
svc, err := gmail.NewService(context.Background(),
|
||||
option.WithoutAuthentication(),
|
||||
option.WithHTTPClient(srv.Client()),
|
||||
option.WithEndpoint(srv.URL+"/"),
|
||||
)
|
||||
if err != nil {
|
||||
t.Fatalf("NewService: %v", err)
|
||||
}
|
||||
|
||||
origNew := newGmailService
|
||||
t.Cleanup(func() { newGmailService = origNew })
|
||||
newGmailService = func(context.Context, string) (*gmail.Service, error) { return svc, nil }
|
||||
}
|
||||
|
||||
func newLabelsDeleteContext(t *testing.T, jsonMode bool) context.Context {
|
||||
t.Helper()
|
||||
|
||||
u, err := ui.New(ui.Options{Stdout: io.Discard, Stderr: io.Discard, Color: "never"})
|
||||
if err != nil {
|
||||
t.Fatalf("ui.New: %v", err)
|
||||
}
|
||||
ctx := ui.WithUI(context.Background(), u)
|
||||
if jsonMode {
|
||||
ctx = outfmt.WithMode(ctx, outfmt.Mode{JSON: true})
|
||||
}
|
||||
return ctx
|
||||
}
|
||||
|
||||
func isLabelsListPath(path string) bool {
|
||||
return strings.HasSuffix(path, "/users/me/labels") || strings.HasSuffix(path, "/gmail/v1/users/me/labels")
|
||||
}
|
||||
|
||||
func isLabelsItemPath(path string) bool {
|
||||
return (strings.Contains(path, "/users/me/labels/") || strings.Contains(path, "/gmail/v1/users/me/labels/")) && !isLabelsListPath(path)
|
||||
}
|
||||
|
||||
func pathTail(path string) string {
|
||||
idx := strings.LastIndex(path, "/")
|
||||
if idx == -1 {
|
||||
return path
|
||||
}
|
||||
return path[idx+1:]
|
||||
}
|
||||
|
||||
func TestGmailLabelsDeleteCmd_JSON_ExactID(t *testing.T) {
|
||||
deleteCalled := false
|
||||
listCalled := false
|
||||
|
||||
newLabelsDeleteService(t, func(w http.ResponseWriter, r *http.Request) {
|
||||
switch {
|
||||
case r.Method == http.MethodGet && isLabelsItemPath(r.URL.Path):
|
||||
if pathTail(r.URL.Path) != "Label_123" {
|
||||
http.NotFound(w, r)
|
||||
return
|
||||
}
|
||||
w.Header().Set("Content-Type", "application/json")
|
||||
_ = json.NewEncoder(w).Encode(map[string]any{"id": "Label_123", "name": "My Label", "type": "user"})
|
||||
return
|
||||
case r.Method == http.MethodGet && isLabelsListPath(r.URL.Path):
|
||||
listCalled = true
|
||||
http.Error(w, "list should not be called", http.StatusInternalServerError)
|
||||
return
|
||||
case r.Method == http.MethodDelete && isLabelsItemPath(r.URL.Path):
|
||||
deleteCalled = true
|
||||
if pathTail(r.URL.Path) != "Label_123" {
|
||||
http.Error(w, "wrong delete id", http.StatusBadRequest)
|
||||
return
|
||||
}
|
||||
w.Header().Set("Content-Type", "application/json")
|
||||
_ = json.NewEncoder(w).Encode(map[string]any{})
|
||||
return
|
||||
default:
|
||||
http.NotFound(w, r)
|
||||
}
|
||||
})
|
||||
|
||||
flags := &RootFlags{Account: "a@b.com", Force: true}
|
||||
ctx := newLabelsDeleteContext(t, true)
|
||||
|
||||
out := captureStdout(t, func() {
|
||||
if err := runKong(t, &GmailLabelsDeleteCmd{}, []string{"Label_123"}, ctx, flags); err != nil {
|
||||
t.Fatalf("execute: %v", err)
|
||||
}
|
||||
})
|
||||
|
||||
if listCalled {
|
||||
t.Fatal("unexpected list call")
|
||||
}
|
||||
if !deleteCalled {
|
||||
t.Fatal("expected delete call")
|
||||
}
|
||||
|
||||
var parsed struct {
|
||||
Deleted bool `json:"deleted"`
|
||||
ID string `json:"id"`
|
||||
Name string `json:"name"`
|
||||
}
|
||||
if err := json.Unmarshal([]byte(out), &parsed); err != nil {
|
||||
t.Fatalf("json parse: %v\nout=%q", err, out)
|
||||
}
|
||||
if !parsed.Deleted || parsed.ID != "Label_123" || parsed.Name != "My Label" {
|
||||
t.Fatalf("unexpected output: %#v", parsed)
|
||||
}
|
||||
}
|
||||
|
||||
func TestGmailLabelsDeleteCmd_NameFallback(t *testing.T) {
|
||||
deleteCalled := false
|
||||
listCalled := false
|
||||
getByIDCalled := false
|
||||
|
||||
newLabelsDeleteService(t, func(w http.ResponseWriter, r *http.Request) {
|
||||
switch {
|
||||
case r.Method == http.MethodGet && isLabelsItemPath(r.URL.Path):
|
||||
id := pathTail(r.URL.Path)
|
||||
if id == "custom" {
|
||||
w.Header().Set("Content-Type", "application/json")
|
||||
w.WriteHeader(http.StatusNotFound)
|
||||
_ = json.NewEncoder(w).Encode(map[string]any{"error": map[string]any{"code": 404, "message": "Requested entity was not found."}})
|
||||
return
|
||||
}
|
||||
if id != "Label_9" {
|
||||
http.NotFound(w, r)
|
||||
return
|
||||
}
|
||||
getByIDCalled = true
|
||||
w.Header().Set("Content-Type", "application/json")
|
||||
_ = json.NewEncoder(w).Encode(map[string]any{"id": "Label_9", "name": "Custom", "type": "user"})
|
||||
return
|
||||
case r.Method == http.MethodGet && isLabelsListPath(r.URL.Path):
|
||||
listCalled = true
|
||||
w.Header().Set("Content-Type", "application/json")
|
||||
_ = json.NewEncoder(w).Encode(map[string]any{"labels": []map[string]any{{"id": "Label_9", "name": "Custom", "type": "user"}}})
|
||||
return
|
||||
case r.Method == http.MethodDelete && isLabelsItemPath(r.URL.Path):
|
||||
deleteCalled = true
|
||||
if pathTail(r.URL.Path) != "Label_9" {
|
||||
http.Error(w, "wrong delete id", http.StatusBadRequest)
|
||||
return
|
||||
}
|
||||
w.Header().Set("Content-Type", "application/json")
|
||||
_ = json.NewEncoder(w).Encode(map[string]any{})
|
||||
return
|
||||
default:
|
||||
http.NotFound(w, r)
|
||||
}
|
||||
})
|
||||
|
||||
flags := &RootFlags{Account: "a@b.com", Force: true}
|
||||
ctx := newLabelsDeleteContext(t, false)
|
||||
if err := runKong(t, &GmailLabelsDeleteCmd{}, []string{"custom"}, ctx, flags); err != nil {
|
||||
t.Fatalf("execute: %v", err)
|
||||
}
|
||||
if !listCalled {
|
||||
t.Fatal("expected list call")
|
||||
}
|
||||
if !getByIDCalled {
|
||||
t.Fatal("expected follow-up get by resolved ID")
|
||||
}
|
||||
if !deleteCalled {
|
||||
t.Fatal("expected delete call")
|
||||
}
|
||||
}
|
||||
|
||||
func TestGmailLabelsDeleteCmd_WrongCaseIDDoesNotResolveAsIDAlias(t *testing.T) {
|
||||
deleteCalled := false
|
||||
getByIDCalled := false
|
||||
|
||||
newLabelsDeleteService(t, func(w http.ResponseWriter, r *http.Request) {
|
||||
switch {
|
||||
case r.Method == http.MethodGet && isLabelsItemPath(r.URL.Path):
|
||||
id := pathTail(r.URL.Path)
|
||||
if id == "label_777" {
|
||||
w.Header().Set("Content-Type", "application/json")
|
||||
w.WriteHeader(http.StatusNotFound)
|
||||
_ = json.NewEncoder(w).Encode(map[string]any{"error": map[string]any{"code": 404, "message": "Requested entity was not found."}})
|
||||
return
|
||||
}
|
||||
if id == "Label_777" {
|
||||
getByIDCalled = true
|
||||
w.Header().Set("Content-Type", "application/json")
|
||||
_ = json.NewEncoder(w).Encode(map[string]any{"id": "Label_777", "name": "Some Label", "type": "user"})
|
||||
return
|
||||
}
|
||||
http.NotFound(w, r)
|
||||
return
|
||||
case r.Method == http.MethodGet && isLabelsListPath(r.URL.Path):
|
||||
w.Header().Set("Content-Type", "application/json")
|
||||
_ = json.NewEncoder(w).Encode(map[string]any{"labels": []map[string]any{{"id": "Label_777", "name": "Some Label", "type": "user"}}})
|
||||
return
|
||||
case r.Method == http.MethodDelete && isLabelsItemPath(r.URL.Path):
|
||||
deleteCalled = true
|
||||
w.Header().Set("Content-Type", "application/json")
|
||||
_ = json.NewEncoder(w).Encode(map[string]any{})
|
||||
return
|
||||
default:
|
||||
http.NotFound(w, r)
|
||||
}
|
||||
})
|
||||
|
||||
flags := &RootFlags{Account: "a@b.com", Force: true}
|
||||
ctx := newLabelsDeleteContext(t, false)
|
||||
|
||||
err := runKong(t, &GmailLabelsDeleteCmd{}, []string{"label_777"}, ctx, flags)
|
||||
if err == nil {
|
||||
t.Fatal("expected not found error")
|
||||
}
|
||||
if !strings.Contains(err.Error(), "label not found: label_777") {
|
||||
t.Fatalf("unexpected error: %v", err)
|
||||
}
|
||||
if getByIDCalled {
|
||||
t.Fatal("wrong-case ID should not resolve to exact ID")
|
||||
}
|
||||
if deleteCalled {
|
||||
t.Fatal("delete should not run")
|
||||
}
|
||||
}
|
||||
|
||||
func TestGmailLabelsDeleteCmd_NoFallbackOnNonNotFound(t *testing.T) {
|
||||
listCalled := false
|
||||
|
||||
newLabelsDeleteService(t, func(w http.ResponseWriter, r *http.Request) {
|
||||
switch {
|
||||
case r.Method == http.MethodGet && isLabelsItemPath(r.URL.Path):
|
||||
w.Header().Set("Content-Type", "application/json")
|
||||
w.WriteHeader(http.StatusForbidden)
|
||||
_ = json.NewEncoder(w).Encode(map[string]any{"error": map[string]any{"code": 403, "message": "forbidden"}})
|
||||
return
|
||||
case r.Method == http.MethodGet && isLabelsListPath(r.URL.Path):
|
||||
listCalled = true
|
||||
http.Error(w, "list should not be called", http.StatusInternalServerError)
|
||||
return
|
||||
default:
|
||||
http.NotFound(w, r)
|
||||
}
|
||||
})
|
||||
|
||||
flags := &RootFlags{Account: "a@b.com", Force: true}
|
||||
ctx := newLabelsDeleteContext(t, false)
|
||||
if err := runKong(t, &GmailLabelsDeleteCmd{}, []string{"Label_403"}, ctx, flags); err == nil {
|
||||
t.Fatal("expected error")
|
||||
} else if !strings.Contains(strings.ToLower(err.Error()), "forbidden") {
|
||||
t.Fatalf("unexpected error: %v", err)
|
||||
}
|
||||
if listCalled {
|
||||
t.Fatal("unexpected list fallback")
|
||||
}
|
||||
}
|
||||
|
||||
func TestGmailLabelsDeleteCmd_SystemLabelBlocked(t *testing.T) {
|
||||
deleteCalled := false
|
||||
|
||||
newLabelsDeleteService(t, func(w http.ResponseWriter, r *http.Request) {
|
||||
switch {
|
||||
case r.Method == http.MethodGet && isLabelsItemPath(r.URL.Path):
|
||||
w.Header().Set("Content-Type", "application/json")
|
||||
_ = json.NewEncoder(w).Encode(map[string]any{"id": "INBOX", "name": "INBOX", "type": "system"})
|
||||
return
|
||||
case r.Method == http.MethodDelete && isLabelsItemPath(r.URL.Path):
|
||||
deleteCalled = true
|
||||
w.Header().Set("Content-Type", "application/json")
|
||||
_ = json.NewEncoder(w).Encode(map[string]any{})
|
||||
return
|
||||
default:
|
||||
http.NotFound(w, r)
|
||||
}
|
||||
})
|
||||
|
||||
flags := &RootFlags{Account: "a@b.com", Force: true}
|
||||
ctx := newLabelsDeleteContext(t, false)
|
||||
err := runKong(t, &GmailLabelsDeleteCmd{}, []string{"INBOX"}, ctx, flags)
|
||||
if err == nil {
|
||||
t.Fatal("expected error")
|
||||
}
|
||||
if !strings.Contains(err.Error(), `cannot delete system label "INBOX"`) {
|
||||
t.Fatalf("unexpected error: %v", err)
|
||||
}
|
||||
if deleteCalled {
|
||||
t.Fatal("delete should not run for system labels")
|
||||
}
|
||||
}
|
||||
Loading…
Reference in New Issue
Block a user