Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
153 changes: 153 additions & 0 deletions shortcuts/drive/drive_folder_permission_get.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,153 @@
// Copyright (c) 2026 Lark Technologies Pte. Ltd.
// SPDX-License-Identifier: MIT

package drive

import (
"context"
"fmt"
"io"
"strings"

"github.com/larksuite/cli/errs"
"github.com/larksuite/cli/internal/validate"
"github.com/larksuite/cli/shortcuts/common"
)

type driveFolderPermissionGetSpec struct {
FolderToken string
}

func readDriveFolderPermissionGetSpec(runtime *common.RuntimeContext) (driveFolderPermissionGetSpec, error) {
rawURL := strings.TrimSpace(runtime.Str("url"))
rawToken := strings.TrimSpace(runtime.Str("folder-token"))

if rawURL == "" && rawToken == "" {
return driveFolderPermissionGetSpec{}, errs.NewValidationError(
errs.SubtypeInvalidArgument,
"pass exactly one of --url or --folder-token",
).WithParam("--url")
}
if rawURL != "" && rawToken != "" {
return driveFolderPermissionGetSpec{}, errs.NewValidationError(
errs.SubtypeInvalidArgument,
"--url and --folder-token are mutually exclusive; pass only one folder locator",
).WithParam("--url")
}

if rawToken != "" {
if err := validate.ResourceName(rawToken, "--folder-token"); err != nil {
return driveFolderPermissionGetSpec{}, errs.NewValidationError(errs.SubtypeInvalidArgument, "%s", err).WithParam("--folder-token")
}
return driveFolderPermissionGetSpec{FolderToken: rawToken}, nil
}

ref, ok := common.ParseResourceURL(rawURL)
if !ok {
return driveFolderPermissionGetSpec{}, errs.NewValidationError(
errs.SubtypeInvalidArgument,
"unsupported --url %q: pass a recognized Lark Drive folder URL such as https://example.feishu.cn/drive/folder/<folder_token>",
rawURL,
).WithParam("--url")
}
if ref.Type != "folder" {
return driveFolderPermissionGetSpec{}, errs.NewValidationError(
errs.SubtypeInvalidArgument,
"--url must point to a Drive folder; got resource type %q",
ref.Type,
).WithParam("--url")
}
if err := validate.ResourceName(ref.Token, "--url"); err != nil {
return driveFolderPermissionGetSpec{}, errs.NewValidationError(errs.SubtypeInvalidArgument, "%s", err).WithParam("--url")
}
return driveFolderPermissionGetSpec{FolderToken: ref.Token}, nil
}
Comment on lines +21 to +64

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win

Move recovery guidance into .WithHint(...).

Several validation errors embed recovery guidance directly in the message instead of using .WithHint(...) (e.g., "pass exactly one of --url or --folder-token", "pass a recognized Lark Drive folder URL such as..."). Per coding guidelines, the message/param should identify what failed, and actionable guidance belongs in .WithHint(...).

♻️ Example fix
 	if rawURL == "" && rawToken == "" {
 		return driveFolderPermissionGetSpec{}, errs.NewValidationError(
 			errs.SubtypeInvalidArgument,
-			"pass exactly one of --url or --folder-token",
-		).WithParam("--url")
+			"missing folder locator",
+		).WithParam("--url").WithHint("pass exactly one of --url or --folder-token")
 	}

Apply similarly to the mutually-exclusive and unsupported/non-folder URL errors.

As per coding guidelines, "param field in errors should only name the user input that actually failed; recovery guidance goes in .WithHint(...)".

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@shortcuts/drive/drive_folder_permission_get.go` around lines 21 - 64, The
validation errors in readDriveFolderPermissionGetSpec mix the failing input with
recovery guidance in the message, which should be moved to .WithHint(...).
Update the checks around rawURL/rawToken, the unsupported URL case, and the
non-folder ref.Type branch so the error message only states what failed and
.WithHint(...) carries guidance like which flag to pass or what folder URL
format is accepted; keep the param on the actual failing input such as --url or
--folder-token.

Source: Coding guidelines


func (s driveFolderPermissionGetSpec) url(runtime *common.RuntimeContext) string {
if runtime != nil && runtime.Config != nil {
if u := common.BuildResourceURL(runtime.Config.Brand, "folder", s.FolderToken); u != "" {
return u
}
}
return common.BuildResourceURL("", "folder", s.FolderToken)
}

func (s driveFolderPermissionGetSpec) params() map[string]interface{} {
return map[string]interface{}{"type": "folder"}
}

func (s driveFolderPermissionGetSpec) apiPath() string {
return drivePermissionPublicV2Path(s.FolderToken)
}

func drivePermissionPublicV2Path(token string) string {
return fmt.Sprintf("/open-apis/drive/v2/permissions/%s/public", validate.EncodePathSegment(token))
}

func (s driveFolderPermissionGetSpec) output(runtime *common.RuntimeContext, data map[string]interface{}) map[string]interface{} {
permissionPublic := interface{}(data)
if nestedPermissionPublic := common.GetMap(data, "permission_public"); nestedPermissionPublic != nil {
permissionPublic = nestedPermissionPublic
}
return map[string]interface{}{
"permission_public": permissionPublic,
}
}
Comment on lines +87 to +95

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

printf '%s\n' '--- file: shortcuts/drive/drive_folder_permission_get.go ---'
cat -n shortcuts/drive/drive_folder_permission_get.go | sed -n '1,180p'

printf '\n%s\n' '--- search for permission_public usage ---'
rg -n "permission_public|driveFolderPermissionGetSpec|output\\(" shortcuts/drive -S

printf '\n%s\n' '--- relevant tests ---'
cat -n shortcuts/drive/drive_folder_permission_get_test.go | sed -n '1,260p'

Repository: larksuite/cli

Length of output: 17452


🏁 Script executed:

#!/bin/bash
set -euo pipefail

printf '%s\n' '--- common.GetMap ---'
rg -n "func GetMap|GetMap\\(" shortcuts internal -S

printf '\n%s\n' '--- similar output() patterns in shortcuts ---'
rg -n "permission_public|interface{}\\(data\\)|nested.*GetMap" shortcuts -S

printf '\n%s\n' '--- drive permission-related shortcuts ---'
fd -a 'drive_*permission*go' shortcuts

printf '\n%s\n' '--- read any closely related implementations ---'
for f in \
  shortcuts/drive/drive_folder_permission_get.go \
  shortcuts/drive/drive_folder_permission_list.go \
  shortcuts/drive/drive_file_permission_get.go \
  shortcuts/drive/drive_file_permission_list.go
do
  [ -f "$f" ] && { echo "### $f"; cat -n "$f" | sed -n '1,220p'; }
done

Repository: larksuite/cli

Length of output: 7948


🏁 Script executed:

#!/bin/bash
set -euo pipefail

printf '%s\n' '--- shortcuts/common/extract.go ---'
cat -n shortcuts/common/extract.go | sed -n '1,140p'

printf '\n%s\n' '--- drive/doc/app output patterns ---'
for f in \
  shortcuts/doc/doc_resource_cover.go \
  shortcuts/apps/apps_openapi_key_get.go \
  shortcuts/apps/apps_openapi_key_create.go \
  shortcuts/drive/drive_member_add.go
do
  if [ -f "$f" ]; then
    echo "### $f"
    cat -n "$f" | sed -n '1,220p'
    echo
  fi
done

printf '\n%s\n' '--- search for "return map[string]interface{}{\"permission_public\": data}"-style fallbacks ---'
rg -n "return map\\[string\\]interface\\{\\}.*data|permission_public" shortcuts -U -S

Repository: larksuite/cli

Length of output: 32891


🏁 Script executed:

#!/bin/bash
set -euo pipefail

printf '%s\n' '--- search for drive permission public API docs / response shape ---'
rg -n "permission_public|permissions/.*/public|drive/v2/permissions" . -S

printf '\n%s\n' '--- inspect any docs or comments around the shortcut ---'
for f in \
  shortcuts/drive/drive_folder_permission_get.go \
  shortcuts/drive/drive_folder_permission_get_test.go \
  README.md
do
  if [ -f "$f" ]; then
    echo "### $f"
    cat -n "$f" | sed -n '1,260p'
    echo
  fi
done

Repository: larksuite/cli

Length of output: 33248


🏁 Script executed:

#!/bin/bash
set -euo pipefail

printf '%s\n' '--- CallAPITyped implementation and callers ---'
rg -n "func .*CallAPITyped|CallAPITyped\\(" shortcuts internal errs -S

printf '\n%s\n' '--- RuntimeContext / typed API response handling ---'
for f in \
  shortcuts/common/runtime.go \
  shortcuts/common/runtime_test.go \
  internal/httpmock/*.go
do
  [ -f "$f" ] && { echo "### $f"; cat -n "$f" | sed -n '1,260p'; echo; }
done

Repository: larksuite/cli

Length of output: 47313


Avoid falling back to the full response map

If permission_public is missing, this should not return the entire data payload as its value. That reshapes the output, can surface unrelated top-level fields, and hides a schema mismatch; return only the nested object (or fail/leave it empty) instead. Add a test for the missing-key case.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@shortcuts/drive/drive_folder_permission_get.go` around lines 87 - 95, The
driveFolderPermissionGetSpec.output method is falling back to the entire data
map when permission_public is missing, which reshapes the output unexpectedly;
update output to return only the nested permission_public object (or an
empty/absent value) instead of data, and keep the behavior localized to
driveFolderPermissionGetSpec.output and common.GetMap handling. Add a test
covering the missing permission_public case to verify the response does not
expose unrelated top-level fields.


// DriveFolderPermissionGet queries the permission_public settings for a Drive
// folder itself.
var DriveFolderPermissionGet = common.Shortcut{
Service: "drive",
Command: "+folder-permission-get",
Description: "Get a Drive folder's sharing, copy, download, and comment permission settings",
Risk: "read",
AuthTypes: []string{"user", "bot"},
HasFormat: true,
Flags: []common.Flag{
{Name: "url", Desc: "Drive folder URL, for example https://example.feishu.cn/drive/folder/<folder_token>"},
{Name: "folder-token", Desc: "Drive folder token; mutually exclusive with --url"},
},
Tips: []string{
"Pass exactly one of --url or --folder-token.",
"This shortcut reads the folder's own permission settings; it does not list child document permissions.",
},
Validate: func(ctx context.Context, runtime *common.RuntimeContext) error {
_, err := readDriveFolderPermissionGetSpec(runtime)
return err
},
DryRun: func(ctx context.Context, runtime *common.RuntimeContext) *common.DryRunAPI {
spec, err := readDriveFolderPermissionGetSpec(runtime)
if err != nil {
return common.NewDryRunAPI().Set("error", err.Error())
}
return common.NewDryRunAPI().
Desc("Get Drive folder permission settings").
GET(spec.apiPath()).
Params(spec.params())
},
Execute: func(ctx context.Context, runtime *common.RuntimeContext) error {
spec, err := readDriveFolderPermissionGetSpec(runtime)
if err != nil {
return err
}

fmt.Fprintf(runtime.IO().ErrOut, "Getting permission settings for folder %s...\n", common.MaskToken(spec.FolderToken))
data, err := runtime.CallAPITyped(
"GET",
spec.apiPath(),
spec.params(),
nil,
)
if err != nil {
return err
}

out := spec.output(runtime, data)
runtime.OutFormat(out, nil, func(w io.Writer) {
fmt.Fprintf(w, "Type: folder\n")
fmt.Fprintf(w, "FolderToken: %s\n", spec.FolderToken)
fmt.Fprintf(w, "URL: %s\n", spec.url(runtime))
})
return nil
},
}
221 changes: 221 additions & 0 deletions shortcuts/drive/drive_folder_permission_get_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,221 @@
// Copyright (c) 2026 Lark Technologies Pte. Ltd.
// SPDX-License-Identifier: MIT

package drive

import (
"context"
"encoding/json"
"strings"
"testing"

"github.com/spf13/cobra"

"github.com/larksuite/cli/errs"
"github.com/larksuite/cli/internal/cmdutil"
"github.com/larksuite/cli/internal/httpmock"
"github.com/larksuite/cli/shortcuts/common"
)

func newDriveFolderPermissionGetRuntime(t *testing.T, rawURL, folderToken string) *common.RuntimeContext {
t.Helper()

cmd := &cobra.Command{Use: "drive +folder-permission-get"}
cmd.Flags().String("url", "", "")
cmd.Flags().String("folder-token", "", "")
if rawURL != "" {
if err := cmd.Flags().Set("url", rawURL); err != nil {
t.Fatalf("set --url: %v", err)
}
}
if folderToken != "" {
if err := cmd.Flags().Set("folder-token", folderToken); err != nil {
t.Fatalf("set --folder-token: %v", err)
}
}
return common.TestNewRuntimeContext(cmd, driveTestConfig())
}

func TestDriveFolderPermissionGetSpecResolvesFolderURL(t *testing.T) {
t.Parallel()

runtime := newDriveFolderPermissionGetRuntime(t, "https://example.feishu.cn/drive/folder/fldTok?from=share", "")
spec, err := readDriveFolderPermissionGetSpec(runtime)
if err != nil {
t.Fatalf("read spec: %v", err)
}
if spec.FolderToken != "fldTok" {
t.Fatalf("FolderToken = %q, want fldTok", spec.FolderToken)
}
}

func TestDriveFolderPermissionGetSpecResolvesBareFolderToken(t *testing.T) {
t.Parallel()

runtime := newDriveFolderPermissionGetRuntime(t, "", " fldTok ")
spec, err := readDriveFolderPermissionGetSpec(runtime)
if err != nil {
t.Fatalf("read spec: %v", err)
}
if spec.FolderToken != "fldTok" {
t.Fatalf("FolderToken = %q, want fldTok", spec.FolderToken)
}
}

func TestDriveFolderPermissionGetSpecValidationErrorsAreTyped(t *testing.T) {
t.Parallel()

tests := []struct {
name string
rawURL string
folderToken string
wantParam string
wantMessage string
}{
{
name: "missing locator",
wantParam: "--url",
wantMessage: "pass exactly one",
},
{
name: "mutually exclusive locators",
rawURL: "https://example.feishu.cn/drive/folder/fldTok",
folderToken: "fldTok",
wantParam: "--url",
wantMessage: "mutually exclusive",
},
{
name: "non-folder URL",
rawURL: "https://example.feishu.cn/docx/doxTok",
wantParam: "--url",
wantMessage: "must point to a Drive folder",
},
{
name: "unsupported URL",
rawURL: "https://example.feishu.cn/calendar/calTok",
wantParam: "--url",
wantMessage: "unsupported --url",
},
{
name: "invalid bare folder token",
folderToken: "../bad",
wantParam: "--folder-token",
wantMessage: "--folder-token",
},
}

for _, temp := range tests {
tt := temp
t.Run(tt.name, func(t *testing.T) {
t.Parallel()

runtime := newDriveFolderPermissionGetRuntime(t, tt.rawURL, tt.folderToken)
_, err := readDriveFolderPermissionGetSpec(runtime)
if err == nil {
t.Fatal("expected validation error, got nil")
}
problem, ok := errs.ProblemOf(err)
if !ok {
t.Fatalf("error is not typed: %T %v", err, err)
}
if problem.Category != errs.CategoryValidation || problem.Subtype != errs.SubtypeInvalidArgument {
t.Fatalf("problem = %s/%s, want validation/invalid_argument", problem.Category, problem.Subtype)
}
if validationErr, ok := err.(*errs.ValidationError); ok {
if validationErr.Param != tt.wantParam {
t.Fatalf("param = %q, want %q", validationErr.Param, tt.wantParam)
}
} else {
t.Fatalf("error type = %T, want *errs.ValidationError", err)
}
if !strings.Contains(err.Error(), tt.wantMessage) {
t.Fatalf("error = %q, want substring %q", err.Error(), tt.wantMessage)
}
})
}
}

func TestDriveFolderPermissionGetDryRunIncludesGETRequest(t *testing.T) {
t.Parallel()

runtime := newDriveFolderPermissionGetRuntime(t, "https://example.feishu.cn/drive/folder/fldTok", "")
dry := DriveFolderPermissionGet.DryRun(context.Background(), runtime)
if dry == nil {
t.Fatal("DryRun returned nil")
}
data, err := json.Marshal(dry)
if err != nil {
t.Fatalf("marshal dry-run: %v", err)
}
out := string(data)
for _, want := range []string{
`"/open-apis/drive/v2/permissions/fldTok/public"`,
`"GET"`,
`"type":"folder"`,
} {
if !strings.Contains(out, want) {
t.Fatalf("dry-run output missing %q:\n%s", want, out)
}
}
if strings.Contains(out, `"folder_token"`) {
t.Fatalf("dry-run output contains folder_token, want omitted:\n%s", out)
}
}

func TestDriveFolderPermissionGetExecutePreservesPermissionPublic(t *testing.T) {
f, stdout, _, reg := cmdutil.TestFactory(t, driveTestConfig())
reg.Register(&httpmock.Stub{
Method: "GET",
URL: "/open-apis/drive/v2/permissions/fldTok/public?type=folder",
Body: map[string]interface{}{
"code": 0,
"msg": "ok",
"data": map[string]interface{}{
"permission_public": map[string]interface{}{
"link_share_entity": "closed",
"external_access_entity": "closed",
"security_entity": "anyone_can_view",
"comment_entity": "anyone_can_view",
"share_entity": "anyone",
"manage_collaborator_entity": "collaborator_can_view",
"lock_switch": false,
"server_future_folder_field": "preserved",
},
},
},
})

err := mountAndRunDrive(t, DriveFolderPermissionGet, []string{
"+folder-permission-get",
"--folder-token", "fldTok",
"--as", "bot",
}, f, stdout)
if err != nil {
t.Fatalf("unexpected error: %v", err)
}

data := decodeDriveEnvelope(t, stdout)
for _, key := range []string{"type", "folder_token", "url"} {
if _, ok := data[key]; ok {
t.Fatalf("data[%s] = %#v, want field omitted", key, data[key])
}
}
permissionPublic, _ := data["permission_public"].(map[string]interface{})
if permissionPublic == nil {
t.Fatalf("permission_public missing in output: %#v", data)
}
for key, want := range map[string]interface{}{
"link_share_entity": "closed",
"external_access_entity": "closed",
"security_entity": "anyone_can_view",
"comment_entity": "anyone_can_view",
"share_entity": "anyone",
"manage_collaborator_entity": "collaborator_can_view",
"lock_switch": false,
"server_future_folder_field": "preserved",
} {
if permissionPublic[key] != want {
t.Fatalf("permission_public[%s] = %#v, want %#v", key, permissionPublic[key], want)
}
}
}
1 change: 1 addition & 0 deletions shortcuts/drive/shortcuts.go
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ func Shortcuts() []common.Shortcut {
DriveTaskResult,
DriveApplyPermission,
DriveMemberAdd,
DriveFolderPermissionGet,
DriveSecureLabelList,
DriveSecureLabelUpdate,
DriveSearch,
Expand Down
1 change: 1 addition & 0 deletions shortcuts/drive/shortcuts_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@ func TestShortcutsIncludesExpectedCommands(t *testing.T) {
"+task_result",
"+apply-permission",
"+member-add",
"+folder-permission-get",
"+secure-label-list",
"+secure-label-update",
"+search",
Expand Down
Loading
Loading