Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
17 commits
Select commit Hold shift + click to select a range
0a84f10
feat: add FileIO extension for file transfer abstraction
tuxedomm Apr 2, 2026
2ecd703
feat: migrate download shortcuts to FileIO.Save (Phase 2)
tuxedomm Apr 2, 2026
cd306f6
feat: migrate upload/read-file shortcuts to FileIO.Open/Stat (Phase 3)
tuxedomm Apr 2, 2026
18c43a7
fix: resolve merge conflict markers and remove stale imports
tuxedomm Apr 3, 2026
d372c0d
fix: adapt fileio cherry-picks to refactor plugin base
tuxedomm Apr 7, 2026
f39f91b
refactor: resolve fileio at runtime
tuxedomm Apr 7, 2026
3e48f83
refactor: consolidate file I/O into localfileio and eliminate direct …
tuxedomm Apr 7, 2026
71ea3f4
refactor: replace validate.SafeLocalFlagPath with FileIO-based valida…
tuxedomm Apr 7, 2026
9f7aa78
refactor: make SafeInputPath and SafeLocalFlagPath private in localfi…
tuxedomm Apr 7, 2026
7b6f14d
refactor: privatize AtomicWriteFromReader/safeLocalFlagPath, add vali…
tuxedomm Apr 7, 2026
1cf4b58
refactor: add RuntimeContext.ValidatePath to deduplicate FileIO.Stat …
tuxedomm Apr 7, 2026
bf5d6bd
chore: add lint rules to enforce FileIO boundary in shortcuts
tuxedomm Apr 7, 2026
a4e0fcf
fix: tighten file permissions in LocalFileIO.Save to 0700/0600
tuxedomm Apr 7, 2026
d9dc585
fix: resolve gofmt and forbidigo lint errors
tuxedomm Apr 7, 2026
cb15a09
fix: avoid nil FileIO in parseStringList by inlining comma-split logic
tuxedomm Apr 7, 2026
8867dca
feat: add FileIO.ResolvePath and fix saved_path to return absolute paths
tuxedomm Apr 7, 2026
1c30e74
fix: gofmt runner_jq_test.go
tuxedomm Apr 7, 2026
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
95 changes: 41 additions & 54 deletions .golangci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ linters:
- reassign # checks that package variables are not reassigned
- unconvert # removes unnecessary type conversions
- unused # checks for unused constants, variables, functions and types
- depguard # blocks forbidden package imports
- forbidigo # forbids specific function calls

# To enable later after fixing existing issues:
Expand All @@ -45,6 +46,7 @@ linters:
linters:
- bodyclose
- gocritic
- depguard
- forbidigo
- path-except: (shortcuts/|internal/)
linters:
Expand All @@ -54,67 +56,52 @@ linters:
- forbidigo

settings:
depguard:
rules:
shortcuts-no-vfs:
files:
- "**/shortcuts/**"
deny:
- pkg: "github.com/larksuite/cli/internal/vfs"
desc: >-
shortcuts must not import internal/vfs directly.
Use runtime.FileIO() for file operations or runtime.ValidatePath() for path validation.
- pkg: "github.com/larksuite/cli/internal/vfs/localfileio"
desc: >-
shortcuts must not import internal/vfs/localfileio directly.
Use runtime.FileIO() for file operations or runtime.ValidatePath() for path validation.
Comment on lines +61 to +72
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P1 New depguard rule breaks existing shortcuts files

The rule denies internal/vfs imports from **/shortcuts/**, but two non-test production files that are not in this PR still import it:

  • shortcuts/event/pipeline.govfs.MkdirAll (line 20)
  • shortcuts/mail/mail_watch.govfs.UserHomeDir and vfs.MkdirAll (line 28)

The current FileIO interface only exposes Open, Stat, and Save, so these callers cannot be migrated without first extending the interface. Running golangci-lint on the full codebase will produce new depguard errors on these files.

forbidigo:
forbid:
# ── Filesystem operations: use internal/vfs instead ──
- pattern: os\.Stat\b
msg: "use vfs.Stat() from internal/vfs"
- pattern: os\.Lstat\b
msg: "use vfs.Lstat() from internal/vfs"
- pattern: os\.Open\b
msg: "use vfs.Open() from internal/vfs"
- pattern: os\.OpenFile\b
msg: "use vfs.OpenFile() from internal/vfs"
- pattern: os\.Create\b
msg: "use vfs.OpenFile() from internal/vfs"
- pattern: os\.CreateTemp\b
# ── os: already wrapped in internal/vfs ──
- pattern: os\.(Stat|Lstat|Open|OpenFile|Rename|ReadFile|WriteFile|Getwd|UserHomeDir|ReadDir)\b
msg: "use the corresponding vfs.Xxx() from internal/vfs"
- pattern: os\.(Create|CreateTemp|MkdirTemp)\b
msg: >-
internal/: use vfs.CreateTemp() from internal/vfs.
shortcuts/: avoid temp files entirely — use io.Reader streaming or in-memory buffers instead.
- pattern: os\.Mkdir\b
internal/: use vfs.CreateTemp() or vfs.OpenFile().
shortcuts/: avoid temp files — use io.Reader streaming or in-memory buffers.
- pattern: os\.Mkdir(All)?\b
msg: "use vfs.MkdirAll() from internal/vfs"
- pattern: os\.MkdirAll\b
msg: "use vfs.MkdirAll() from internal/vfs"
- pattern: os\.Remove\b
- pattern: os\.Remove(All)?\b
msg: >-
internal/: use vfs.Remove() from internal/vfs.
shortcuts/: avoid temp files entirely — use io.Reader streaming or in-memory buffers instead.
- pattern: os\.RemoveAll\b
shortcuts/: avoid temp files — use io.Reader streaming or in-memory buffers.
# ── os: not yet in vfs — add to vfs/fs.go first ──
- pattern: os\.(Chdir|Chmod|Chown|Lchown|Chtimes|CopyFS|DirFS|Link|Symlink|Readlink|Truncate|SameFile)\b
msg: "add this function to internal/vfs/fs.go first, then use vfs.Xxx()"
# ── os: IO streams ──
- pattern: os\.Std(in|out|err)\b
msg: "use IOStreams (In/Out/ErrOut) instead of os.Stdin/Stdout/Stderr"
# ── os: process ──
- pattern: os\.Exit\b
msg: >-
Do not use os.Exit in shortcuts/. Return an error instead and let
the caller (cmd layer) decide how to terminate.
# ── filepath: functions that access the filesystem ──
- pattern: filepath\.(EvalSymlinks|Walk|WalkDir|Glob|Abs)\b
msg: >-
internal/: add RemoveAll to internal/vfs/fs.go first, then use vfs.RemoveAll().
shortcuts/: avoid temp files entirely — use io.Reader streaming or in-memory buffers instead.
- pattern: os\.Rename\b
msg: "use vfs.Rename() from internal/vfs"
- pattern: os\.ReadFile\b
msg: "use vfs.ReadFile() from internal/vfs"
- pattern: os\.WriteFile\b
msg: "use vfs.WriteFile() from internal/vfs"
- pattern: os\.ReadDir\b
msg: "add ReadDir to internal/vfs/fs.go first, then use vfs.ReadDir()"
- pattern: os\.Getwd\b
msg: "use vfs.Getwd() from internal/vfs"
- pattern: os\.Chdir\b
msg: "add Chdir to internal/vfs/fs.go first, then use vfs.Chdir()"
- pattern: os\.UserHomeDir\b
msg: "use vfs.UserHomeDir() from internal/vfs"
- pattern: os\.Chmod\b
msg: "add Chmod to internal/vfs/fs.go first, then use vfs.Chmod()"
- pattern: os\.Chown\b
msg: "add Chown to internal/vfs/fs.go first, then use vfs.Chown()"
- pattern: os\.Lchown\b
msg: "add Lchown to internal/vfs/fs.go first, then use vfs.Lchown()"
- pattern: os\.Link\b
msg: "add Link to internal/vfs/fs.go first, then use vfs.Link()"
- pattern: os\.Symlink\b
msg: "add Symlink to internal/vfs/fs.go first, then use vfs.Symlink()"
- pattern: os\.Readlink\b
msg: "add Readlink to internal/vfs/fs.go first, then use vfs.Readlink()"
- pattern: os\.Truncate\b
msg: "add Truncate to internal/vfs/fs.go first, then use vfs.Truncate()"
- pattern: os\.DirFS\b
msg: "add DirFS to internal/vfs/fs.go first, then use vfs.DirFS()"
- pattern: os\.SameFile\b
msg: "add SameFile to internal/vfs/fs.go first, then use vfs.SameFile()"
These filepath functions access the filesystem directly.
internal/: use vfs helpers or localfileio path validation.
shortcuts/: use runtime.ValidatePath() or runtime.FileIO().
# ── IO streams: use IOStreams from cmdutil instead ──
- pattern: os\.Stdin\b
msg: "use IOStreams.In instead of os.Stdin"
Expand Down
1 change: 1 addition & 0 deletions cmd/api/api.go
Original file line number Diff line number Diff line change
Expand Up @@ -207,6 +207,7 @@ func apiRun(opts *APIOptions) error {
JqExpr: opts.JqExpr,
Out: out,
ErrOut: f.IOStreams.ErrOut,
FileIO: f.ResolveFileIO(opts.Ctx),
})
// MarkRaw tells root error handler to skip enrichPermissionError,
// preserving the original API error detail (log_id, troubleshooter, etc.).
Expand Down
1 change: 1 addition & 0 deletions cmd/service/service.go
Original file line number Diff line number Diff line change
Expand Up @@ -250,6 +250,7 @@ func serviceMethodRun(opts *ServiceMethodOptions) error {
JqExpr: opts.JqExpr,
Out: out,
ErrOut: f.IOStreams.ErrOut,
FileIO: f.ResolveFileIO(opts.Ctx),
CheckError: checkErr,
})
}
Expand Down
28 changes: 28 additions & 0 deletions extension/fileio/registry.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
// Copyright (c) 2026 Lark Technologies Pte. Ltd.
// SPDX-License-Identifier: MIT

package fileio

import "sync"

var (
mu sync.Mutex
provider Provider
)

// Register registers a FileIO Provider.
// Later registrations override earlier ones.
// Typically called from init() via blank import.
func Register(p Provider) {
mu.Lock()
defer mu.Unlock()
provider = p
}

// GetProvider returns the currently registered Provider.
// Returns nil if no provider has been registered.
func GetProvider() Provider {
mu.Lock()
defer mu.Unlock()
return provider
}
66 changes: 66 additions & 0 deletions extension/fileio/types.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,66 @@
// Copyright (c) 2026 Lark Technologies Pte. Ltd.
// SPDX-License-Identifier: MIT

package fileio

import (
"context"
"io"
"os"
)

// Provider creates FileIO instances.
// Follows the same API style as extension/credential.Provider.
type Provider interface {
Name() string
ResolveFileIO(ctx context.Context) FileIO
}

// FileIO abstracts file transfer operations for CLI commands.
// The default implementation operates on the local filesystem with
// path validation, directory creation, and atomic writes.
// Inject a custom implementation via Factory.FileIOProvider to replace
// file transfer behavior (e.g. streaming in server mode).
type FileIO interface {
// Open opens a file for reading (upload, attachment, template scenarios).
// The default implementation validates the path via SafeInputPath.
Open(name string) (File, error)

// Stat returns file metadata (size validation, existence checks).
// The default implementation validates the path via SafeInputPath.
// Use os.IsNotExist(err) to distinguish "file not found" from "invalid path".
Stat(name string) (os.FileInfo, error)

// ResolvePath returns the validated, absolute path for the given output path.
// The default implementation delegates to SafeOutputPath.
// Use this to obtain the canonical saved path for user-facing output.
ResolvePath(path string) (string, error)

// Save writes content to the target path and returns a SaveResult.
// The default implementation validates via SafeOutputPath, creates
// parent directories, and writes atomically.
Save(path string, opts SaveOptions, body io.Reader) (SaveResult, error)
}

// File is the interface returned by FileIO.Open.
// It covers the subset of *os.File methods actually used by CLI commands.
// *os.File satisfies this interface without adaptation.
type File interface {
io.Reader
io.ReaderAt
io.Closer
Stat() (os.FileInfo, error)
}

// SaveResult holds the outcome of a Save operation.
type SaveResult interface {
Size() int64 // actual bytes written
}

// SaveOptions carries metadata for Save.
// The default (local) implementation ignores these fields;
// server-mode implementations use them to construct streaming response frames.
type SaveOptions struct {
ContentType string // MIME type
ContentLength int64 // content length; -1 if unknown
}
39 changes: 19 additions & 20 deletions internal/client/response.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,15 +9,13 @@ import (
"fmt"
"io"
"mime"
"path/filepath"
"strings"

larkcore "github.com/larksuite/oapi-sdk-go/v3/core"

"github.com/larksuite/cli/extension/fileio"
"github.com/larksuite/cli/internal/output"
"github.com/larksuite/cli/internal/util"
"github.com/larksuite/cli/internal/validate"
"github.com/larksuite/cli/internal/vfs"
)

// ── Response routing ──
Expand All @@ -29,6 +27,7 @@ type ResponseOptions struct {
JqExpr string // if set, apply jq filter instead of Format
Out io.Writer // stdout
ErrOut io.Writer // stderr
FileIO fileio.FileIO // file transfer abstraction; nil falls back to direct os calls
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🔴 Critical

Avoid a nil FileIO panic on save paths.

ResponseOptions still says nil falls back, but SaveResponse now unconditionally calls fio.Save(...). Any caller using the zero value will panic on --output or on the auto-save binary path instead of getting a normal error.

🩹 Minimal guard
 func SaveResponse(fio fileio.FileIO, resp *larkcore.ApiResp, outputPath string) (map[string]interface{}, error) {
+	if fio == nil {
+		return nil, fmt.Errorf("file I/O not configured")
+	}
 	result, err := fio.Save(outputPath, fileio.SaveOptions{
 		ContentType:   resp.Header.Get("Content-Type"),
 		ContentLength: int64(len(resp.RawBody)),
 	}, bytes.NewReader(resp.RawBody))

Also applies to: 63-63, 77-81, 122-126

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@internal/client/response.go` at line 30, The code unconditionally calls
fio.Save(...) which will panic when ResponseOptions.FileIO is nil; update
SaveResponse and the other save paths (where fio.Save is used) to guard against
nil by resolving a fallback OS-backed FileIO (e.g., create or use a
fileio.NewOSFileIO()/fileio.Default implementation) before calling Save, or
return a clear error if no FileIO is available; locate the ResponseOptions
struct and the SaveResponse (and related save) functions and insert a small
nil-check: if ro.FileIO == nil { ro.FileIO = fileio.NewOSFileIO() } (or assign a
local fio := ro.FileIO; if nil { fio = fileio.NewOSFileIO() }) then call
fio.Save(...) to avoid the panic.

// CheckError is called on parsed JSON results. Nil defaults to CheckLarkResponse.
CheckError func(interface{}) error
}
Expand Down Expand Up @@ -61,7 +60,7 @@ func HandleResponse(resp *larkcore.ApiResp, opts ResponseOptions) error {
return apiErr
}
if opts.OutputPath != "" {
return saveAndPrint(resp, opts.OutputPath, opts.Out)
return saveAndPrint(opts.FileIO, resp, opts.OutputPath, opts.Out)
}
if opts.JqExpr != "" {
return output.JqFilter(opts.Out, result, opts.JqExpr)
Expand All @@ -75,11 +74,11 @@ func HandleResponse(resp *larkcore.ApiResp, opts ResponseOptions) error {
return output.ErrValidation("--jq requires a JSON response (got Content-Type: %s)", ct)
}
if opts.OutputPath != "" {
return saveAndPrint(resp, opts.OutputPath, opts.Out)
return saveAndPrint(opts.FileIO, resp, opts.OutputPath, opts.Out)
}

// No --output: auto-save with derived filename.
meta, err := SaveResponse(resp, ResolveFilename(resp))
meta, err := SaveResponse(opts.FileIO, resp, ResolveFilename(resp))
if err != nil {
return output.Errorf(output.ExitInternal, "file_error", "%s", err)
Comment on lines 82 to 83
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

Preserve validation failures from FileIO.Save.

fio.Save can now return user-input errors such as an invalid --output path, but both save paths remap every failure to file_error/ExitInternal. fmt.Errorf("cannot write file: %s", err) also strips the cause chain, so callers cannot recover the right exit code later. Please keep typed validation errors intact and map them to output.ErrValidation here.

Also applies to: 92-93, 123-129

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@internal/client/response.go` around lines 82 - 83, When handling errors
returned from FileIO.Save (fio.Save) where currently code does return
output.Errorf(output.ExitInternal, "file_error", "%s", err) and also uses
fmt.Errorf("cannot write file: %s", err), change the logic to detect typed
validation errors from fio.Save (use errors.Is / errors.As against the
validation error type returned by the FileIO layer) and, when matched, return
output.Errorf(output.ErrValidation, "file_error", "%v", err) (preserving the
original error rather than wrapping with fmt.Errorf); for non-validation
failures keep the ExitInternal mapping. Apply the same change to the other
similar return sites that currently remap all failures to ExitInternal (the
other occurrences in this file).

}
Expand All @@ -88,8 +87,8 @@ func HandleResponse(resp *larkcore.ApiResp, opts ResponseOptions) error {
return nil
}

func saveAndPrint(resp *larkcore.ApiResp, path string, w io.Writer) error {
meta, err := SaveResponse(resp, path)
func saveAndPrint(fio fileio.FileIO, resp *larkcore.ApiResp, path string, w io.Writer) error {
meta, err := SaveResponse(fio, resp, path)
if err != nil {
return output.Errorf(output.ExitInternal, "file_error", "%s", err)
}
Expand Down Expand Up @@ -119,23 +118,23 @@ func ParseJSONResponse(resp *larkcore.ApiResp) (interface{}, error) {
// ── File saving ──

// SaveResponse writes an API response body to the given outputPath and returns metadata.
func SaveResponse(resp *larkcore.ApiResp, outputPath string) (map[string]interface{}, error) {
safePath, err := validate.SafeOutputPath(outputPath)
// It delegates to FileIO.Save for path validation and atomic write; fio must not be nil.
func SaveResponse(fio fileio.FileIO, resp *larkcore.ApiResp, outputPath string) (map[string]interface{}, error) {
result, err := fio.Save(outputPath, fileio.SaveOptions{
ContentType: resp.Header.Get("Content-Type"),
ContentLength: int64(len(resp.RawBody)),
}, bytes.NewReader(resp.RawBody))
if err != nil {
return nil, fmt.Errorf("unsafe output path: %s", err)
}

if err := vfs.MkdirAll(filepath.Dir(safePath), 0700); err != nil {
return nil, fmt.Errorf("create directory: %s", err)
}

if err := validate.AtomicWrite(safePath, resp.RawBody, 0644); err != nil {
return nil, fmt.Errorf("cannot write file: %s", err)
}

resolvedPath, _ := fio.ResolvePath(outputPath)
if resolvedPath == "" {
resolvedPath = outputPath
}
return map[string]interface{}{
"saved_path": safePath,
"size_bytes": len(resp.RawBody),
"saved_path": resolvedPath,
"size_bytes": result.Size(),
"content_type": resp.Header.Get("Content-Type"),
}, nil
}
Expand Down
Loading
Loading