Skip to content

Commit eea2382

Browse files
fix(security): require confirmation for risky share/delegate/filter actions
1 parent e3d2f4f commit eea2382

8 files changed

Lines changed: 59 additions & 18 deletions

internal/cmd/drive.go

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -779,6 +779,11 @@ func (c *DriveShareCmd) Run(ctx context.Context, flags *RootFlags) error {
779779
if role != drivePermRoleReader && role != drivePermRoleWriter {
780780
return usage("invalid --role (expected reader|writer)")
781781
}
782+
if to == driveShareToAnyone {
783+
if confirmErr := confirmDestructive(ctx, flags, fmt.Sprintf("share drive file %s with anyone (public)", fileID)); confirmErr != nil {
784+
return confirmErr
785+
}
786+
}
782787

783788
svc, err := newDriveService(ctx, account)
784789
if err != nil {

internal/cmd/drive_errors_test.go

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -62,4 +62,9 @@ func TestDriveDeleteUnshare_NoInput(t *testing.T) {
6262
if err := runKong(t, unshareCmd, []string{"file1", "perm1"}, context.Background(), flags); err == nil || !strings.Contains(err.Error(), "refusing") {
6363
t.Fatalf("expected refusing error, got %v", err)
6464
}
65+
66+
shareCmd := &DriveShareCmd{}
67+
if err := runKong(t, shareCmd, []string{"file1", "--to", "anyone"}, context.Background(), flags); err == nil || !strings.Contains(err.Error(), "refusing to share drive file file1 with anyone (public)") {
68+
t.Fatalf("expected refusing error, got %v", err)
69+
}
6570
}

internal/cmd/execute_drive_more_commands_test.go

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -147,11 +147,11 @@ func TestExecute_DriveMoreCommands_JSON(t *testing.T) {
147147
t.Fatalf("move: %v", err)
148148
}
149149
})
150-
_ = captureStdout(t, func() {
151-
if err := Execute([]string{"--json", "--account", "a@b.com", "drive", "share", "id1", "--anyone", "--role", "reader"}); err != nil {
152-
t.Fatalf("share: %v", err)
153-
}
154-
})
150+
_ = captureStdout(t, func() {
151+
if err := Execute([]string{"--json", "--force", "--account", "a@b.com", "drive", "share", "id1", "--anyone", "--role", "reader"}); err != nil {
152+
t.Fatalf("share: %v", err)
153+
}
154+
})
155155
_ = captureStdout(t, func() {
156156
if err := Execute([]string{"--json", "--account", "a@b.com", "drive", "permissions", "id1"}); err != nil {
157157
t.Fatalf("permissions: %v", err)
@@ -292,9 +292,9 @@ func TestExecute_DriveMoreCommands_Text(t *testing.T) {
292292
if err := Execute([]string{"--account", "a@b.com", "drive", "move", "id1", "--parent", "np"}); err != nil {
293293
t.Fatalf("move: %v", err)
294294
}
295-
if err := Execute([]string{"--account", "a@b.com", "drive", "share", "id1", "--anyone", "--role", "reader"}); err != nil {
296-
t.Fatalf("share: %v", err)
297-
}
295+
if err := Execute([]string{"--force", "--account", "a@b.com", "drive", "share", "id1", "--anyone", "--role", "reader"}); err != nil {
296+
t.Fatalf("share: %v", err)
297+
}
298298
if err := Execute([]string{"--account", "a@b.com", "drive", "permissions", "id1"}); err != nil {
299299
t.Fatalf("permissions: %v", err)
300300
}

internal/cmd/execute_gmail_settings_more_commands_test.go

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -216,11 +216,11 @@ func TestExecute_GmailSettingsMoreCommands_JSON(t *testing.T) {
216216
t.Fatalf("delegates get: %v", err)
217217
}
218218
})
219-
_ = captureStdout(t, func() {
220-
if err := Execute([]string{"--json", "--account", "a@b.com", "gmail", "delegates", "add", "d@b.com"}); err != nil {
221-
t.Fatalf("delegates add: %v", err)
222-
}
223-
})
219+
_ = captureStdout(t, func() {
220+
if err := Execute([]string{"--json", "--force", "--account", "a@b.com", "gmail", "delegates", "add", "d@b.com"}); err != nil {
221+
t.Fatalf("delegates add: %v", err)
222+
}
223+
})
224224
_ = captureStdout(t, func() {
225225
if err := Execute([]string{"--json", "--force", "--account", "a@b.com", "gmail", "delegates", "remove", "d@b.com"}); err != nil {
226226
t.Fatalf("delegates remove: %v", err)

internal/cmd/gmail_delegates.go

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -109,6 +109,9 @@ func (c *GmailDelegatesAddCmd) Run(ctx context.Context, flags *RootFlags) error
109109
}); err != nil {
110110
return err
111111
}
112+
if confirmErr := confirmDestructive(ctx, flags, fmt.Sprintf("add gmail delegate %s (grants mailbox read access)", delegateEmail)); confirmErr != nil {
113+
return confirmErr
114+
}
112115

113116
account, err := requireAccount(flags)
114117
if err != nil {

internal/cmd/gmail_delegates_test.go

Lines changed: 14 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,10 @@
11
package cmd
22

3-
import "testing"
3+
import (
4+
"context"
5+
"strings"
6+
"testing"
7+
)
48

59
func TestDelegatesCommandsExist(t *testing.T) {
610
// Unit tests for the actual API calls live in integration; here we just ensure
@@ -11,3 +15,12 @@ func TestDelegatesCommandsExist(t *testing.T) {
1115
_ = GmailDelegatesAddCmd{}
1216
_ = GmailDelegatesRemoveCmd{}
1317
}
18+
19+
func TestGmailDelegatesAdd_NoInputRequiresForce(t *testing.T) {
20+
flags := &RootFlags{Account: "a@b.com", NoInput: true}
21+
cmd := &GmailDelegatesAddCmd{}
22+
err := runKong(t, cmd, []string{"delegate@example.com"}, context.Background(), flags)
23+
if err == nil || !strings.Contains(err.Error(), "refusing to add gmail delegate") {
24+
t.Fatalf("expected refusing error, got %v", err)
25+
}
26+
}

internal/cmd/gmail_filters.go

Lines changed: 10 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -168,14 +168,15 @@ type GmailFiltersCreateCmd struct {
168168

169169
func (c *GmailFiltersCreateCmd) Run(ctx context.Context, flags *RootFlags) error {
170170
u := ui.FromContext(ctx)
171+
forwardTarget := strings.TrimSpace(c.Forward)
171172

172173
// Validate that at least one criteria is specified
173174
if c.From == "" && c.To == "" && c.Subject == "" && c.Query == "" && !c.HasAttachment {
174175
return errors.New("must specify at least one criteria flag (--from, --to, --subject, --query, or --has-attachment)")
175176
}
176177

177178
// Validate that at least one action is specified
178-
if c.AddLabel == "" && c.RemoveLabel == "" && !c.Archive && !c.MarkRead && !c.Star && c.Forward == "" && !c.Trash && !c.NeverSpam && !c.Important {
179+
if c.AddLabel == "" && c.RemoveLabel == "" && !c.Archive && !c.MarkRead && !c.Star && forwardTarget == "" && !c.Trash && !c.NeverSpam && !c.Important {
179180
return errors.New("must specify at least one action flag (--add-label, --remove-label, --archive, --mark-read, --star, --forward, --trash, --never-spam, or --important)")
180181
}
181182

@@ -193,14 +194,19 @@ func (c *GmailFiltersCreateCmd) Run(ctx context.Context, flags *RootFlags) error
193194
"archive": c.Archive,
194195
"mark_read": c.MarkRead,
195196
"star": c.Star,
196-
"forward": strings.TrimSpace(c.Forward),
197+
"forward": forwardTarget,
197198
"trash": c.Trash,
198199
"never_spam": c.NeverSpam,
199200
"important": c.Important,
200201
},
201202
}); err != nil {
202203
return err
203204
}
205+
if forwardTarget != "" {
206+
if confirmErr := confirmDestructive(ctx, flags, fmt.Sprintf("create gmail filter forwarding to %s", forwardTarget)); confirmErr != nil {
207+
return confirmErr
208+
}
209+
}
204210

205211
account, err := requireAccount(flags)
206212
if err != nil {
@@ -278,8 +284,8 @@ func (c *GmailFiltersCreateCmd) Run(ctx context.Context, flags *RootFlags) error
278284
action.AddLabelIds = append(action.AddLabelIds, "STARRED")
279285
}
280286

281-
if c.Forward != "" {
282-
action.Forward = c.Forward
287+
if forwardTarget != "" {
288+
action.Forward = forwardTarget
283289
}
284290

285291
if c.Trash {

internal/cmd/gmail_filters_cmd_test.go

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,15 @@ func TestGmailFiltersCreate_Validation(t *testing.T) {
2929
}
3030
}
3131

32+
func TestGmailFiltersCreate_Forward_NoInputRequiresForce(t *testing.T) {
33+
flags := &RootFlags{Account: "a@b.com", NoInput: true}
34+
cmd := &GmailFiltersCreateCmd{}
35+
err := runKong(t, cmd, []string{"--from", "a@example.com", "--forward", "f@example.com"}, context.Background(), flags)
36+
if err == nil || !strings.Contains(err.Error(), "refusing to create gmail filter forwarding") {
37+
t.Fatalf("expected refusing error, got %v", err)
38+
}
39+
}
40+
3241
func TestGmailFilters_TextPaths(t *testing.T) {
3342
origNew := newGmailService
3443
t.Cleanup(func() { newGmailService = origNew })

0 commit comments

Comments
 (0)