Skip to content
30 changes: 30 additions & 0 deletions shortcuts/common/artifact_path.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
// Copyright (c) 2026 Lark Technologies Pte. Ltd.
// SPDX-License-Identifier: MIT

// This file defines artifact-path conventions shared between
// `minutes +download` and `vc +notes`. Callers outside those two shortcuts
// should not take a dependency on these symbols.

package common

import "path/filepath"

// DefaultMinuteArtifactSubdir is the top-level directory for minute-scoped
// artifacts under the default layout.
const DefaultMinuteArtifactSubdir = "minutes"

// DefaultTranscriptFileName is the fixed transcript filename under the
// default layout. Recording files keep the server-provided name.
const DefaultTranscriptFileName = "transcript.txt"

// ArtifactTypeRecording is the artifact_type value emitted by
// `minutes +download` so that callers can index results by kind without
// parsing saved_path.
const ArtifactTypeRecording = "recording"

// DefaultMinuteArtifactDir returns the default output directory for an
// artifact keyed by minuteToken. The same path is shared across commands so
// that related artifacts of one meeting land together.
func DefaultMinuteArtifactDir(minuteToken string) string {
return filepath.Join(DefaultMinuteArtifactSubdir, minuteToken)

Check warning on line 29 in shortcuts/common/artifact_path.go

View check run for this annotation

Codecov / codecov/patch

shortcuts/common/artifact_path.go#L28-L29

Added lines #L28 - L29 were not covered by tests
}
13 changes: 6 additions & 7 deletions shortcuts/common/validate.go
Original file line number Diff line number Diff line change
Expand Up @@ -83,13 +83,12 @@ func ParseIntBounded(rt *RuntimeContext, name string, min, max int) int {
return v
}

// ValidateSafeOutputDir ensures outputDir is a relative path that resolves
// within the current working directory, preventing path traversal attacks
// (including symlink-based escape).
// It delegates all validation to FileIO.ResolvePath which already performs
// cwd-boundary checks, symlink resolution, and control-character rejection.
func ValidateSafeOutputDir(fio fileio.FileIO, outputDir string) error {
_, err := fio.ResolvePath(outputDir)
// ValidateSafePath ensures path is relative and resolves within the current
// working directory. It catches traversal, symlink escape, and control
// characters by delegating to FileIO.ResolvePath. Works for both file and
// directory paths.
func ValidateSafePath(fio fileio.FileIO, path string) error {
_, err := fio.ResolvePath(path)
return err
}

Expand Down
26 changes: 13 additions & 13 deletions shortcuts/common/validate_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -172,7 +172,7 @@ func TestParseIntBounded(t *testing.T) {
}

// ---------------------------------------------------------------------------
// ValidateSafeOutputDir — symlink escape prevention
// ValidateSafePath — symlink escape prevention
// ---------------------------------------------------------------------------

// chdirForTest changes CWD to dir and restores the original CWD on cleanup.
Expand All @@ -188,9 +188,9 @@ func chdirForTest(t *testing.T, dir string) {
t.Cleanup(func() { os.Chdir(orig) })
}

// TestValidateSafeOutputDir_RejectsSymlinkEscape verifies that a relative path
// TestValidateSafePath_RejectsSymlinkEscape verifies that a relative path
// that resolves to a symlink pointing outside CWD is rejected.
func TestValidateSafeOutputDir_RejectsSymlinkEscape(t *testing.T) {
func TestValidateSafePath_RejectsSymlinkEscape(t *testing.T) {
outside := t.TempDir() // target outside CWD
workDir := t.TempDir()
chdirForTest(t, workDir)
Expand All @@ -200,29 +200,29 @@ func TestValidateSafeOutputDir_RejectsSymlinkEscape(t *testing.T) {
t.Fatalf("Symlink: %v", err)
}

if err := ValidateSafeOutputDir(&localfileio.LocalFileIO{}, "evil_out"); err == nil {
if err := ValidateSafePath(&localfileio.LocalFileIO{}, "evil_out"); err == nil {
t.Fatal("expected error for symlink pointing outside CWD, got nil")
}
}

// TestValidateSafeOutputDir_RejectsDanglingSymlink verifies that a dangling
// TestValidateSafePath_RejectsDanglingSymlink verifies that a dangling
// symlink (target does not exist) is rejected to prevent future escapes.
func TestValidateSafeOutputDir_RejectsDanglingSymlink(t *testing.T) {
func TestValidateSafePath_RejectsDanglingSymlink(t *testing.T) {
workDir := t.TempDir()
chdirForTest(t, workDir)

if err := os.Symlink("/nonexistent/outside/target", filepath.Join(workDir, "dangling")); err != nil {
t.Fatalf("Symlink: %v", err)
}

if err := ValidateSafeOutputDir(&localfileio.LocalFileIO{}, "dangling"); err == nil {
if err := ValidateSafePath(&localfileio.LocalFileIO{}, "dangling"); err == nil {
t.Fatal("expected error for dangling symlink, got nil")
}
}

// TestValidateSafeOutputDir_AllowsNormalSubdir verifies that an existing real
// TestValidateSafePath_AllowsNormalSubdir verifies that an existing real
// subdirectory within CWD is accepted.
func TestValidateSafeOutputDir_AllowsNormalSubdir(t *testing.T) {
func TestValidateSafePath_AllowsNormalSubdir(t *testing.T) {
workDir := t.TempDir()
chdirForTest(t, workDir)

Expand All @@ -231,18 +231,18 @@ func TestValidateSafeOutputDir_AllowsNormalSubdir(t *testing.T) {
t.Fatalf("Mkdir: %v", err)
}

if err := ValidateSafeOutputDir(&localfileio.LocalFileIO{}, "output"); err != nil {
if err := ValidateSafePath(&localfileio.LocalFileIO{}, "output"); err != nil {
t.Fatalf("expected no error for real subdir, got: %v", err)
}
}

// TestValidateSafeOutputDir_AllowsNonExistentPath verifies that a path that
// TestValidateSafePath_AllowsNonExistentPath verifies that a path that
// does not yet exist (new output directory) is accepted.
func TestValidateSafeOutputDir_AllowsNonExistentPath(t *testing.T) {
func TestValidateSafePath_AllowsNonExistentPath(t *testing.T) {
workDir := t.TempDir()
chdirForTest(t, workDir)

if err := ValidateSafeOutputDir(&localfileio.LocalFileIO{}, "new_output_dir"); err != nil {
if err := ValidateSafePath(&localfileio.LocalFileIO{}, "new_output_dir"); err != nil {
t.Fatalf("expected no error for non-existent path, got: %v", err)
}
}
121 changes: 99 additions & 22 deletions shortcuts/minutes/minutes_download.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,10 +5,13 @@

import (
"context"
"errors"
"fmt"
"io"
"io/fs"
"mime"
"net/http"
"path"
"path/filepath"
"regexp"
"strings"
Expand Down Expand Up @@ -43,7 +46,8 @@
HasFormat: true,
Flags: []common.Flag{
{Name: "minute-tokens", Desc: "minute tokens, comma-separated for batch download (max 50)", Required: true},
{Name: "output", Desc: "output path: file path for single token, directory for batch (default: current dir)"},
{Name: "output", Desc: "output file path (single token)"},
{Name: "output-dir", Desc: "output directory (default: ./minutes/{minute_token}/)"},
{Name: "overwrite", Type: "bool", Desc: "overwrite existing output file"},
{Name: "url-only", Type: "bool", Desc: "only print the download URL(s) without downloading"},
},
Expand All @@ -60,6 +64,22 @@
return output.ErrValidation("invalid minute token %q: must contain only lowercase alphanumeric characters (e.g. obcnq3b9jl72l83w4f149w9c)", token)
}
}
// Cheap checks first, then path-safety resolution.
out := runtime.Str("output")
outDir := runtime.Str("output-dir")
if out != "" && outDir != "" {
return output.ErrValidation("--output and --output-dir cannot both be set")
}
if out != "" {
if err := common.ValidateSafePath(runtime.FileIO(), out); err != nil {
return err
}
}
if outDir != "" {
if err := common.ValidateSafePath(runtime.FileIO(), outDir); err != nil {
return err
}
}
return nil
},
DryRun: func(ctx context.Context, runtime *common.RuntimeContext) *common.DryRunAPI {
Expand All @@ -70,29 +90,53 @@
},
Execute: func(ctx context.Context, runtime *common.RuntimeContext) error {
tokens := common.SplitCSV(runtime.Str("minute-tokens"))
outputPath := runtime.Str("output")
rawOutput := runtime.Str("output")
rawOutputDir := runtime.Str("output-dir")
overwrite := runtime.Bool("overwrite")
urlOnly := runtime.Bool("url-only")
errOut := runtime.IO().ErrOut
single := len(tokens) == 1

// Batch mode: --output must be a directory, not an existing file.
if !single && outputPath != "" {
if fi, err := runtime.FileIO().Stat(outputPath); err == nil && !fi.IsDir() {
return output.ErrValidation("--output %q is a file; batch mode expects a directory path", outputPath)
// Re-interpret --output based on what the path points to. An existing
// directory is promoted to --output-dir so single-token cp semantics
// work. An existing file is rejected in batch mode (the flag carries
// directory semantics there). Unknown filesystem errors are surfaced
// eagerly rather than deferred to Save.
explicitOutputPath := rawOutput
explicitOutputDir := rawOutputDir
if explicitOutputPath != "" {
fi, statErr := runtime.FileIO().Stat(explicitOutputPath)
switch {
case statErr == nil && fi.IsDir():
explicitOutputDir = explicitOutputPath
explicitOutputPath = ""
case statErr == nil && !fi.IsDir():
if !single {
return output.ErrValidation("--output %q is a file; batch mode expects a directory (use --output-dir)", explicitOutputPath)
}
case errors.Is(statErr, fs.ErrNotExist):
if !single {
explicitOutputDir = explicitOutputPath
explicitOutputPath = ""
}
default:
return output.Errorf(output.ExitAPI, "io_error", "cannot access --output %q: %s", explicitOutputPath, statErr)

Check warning on line 123 in shortcuts/minutes/minutes_download.go

View check run for this annotation

Codecov / codecov/patch

shortcuts/minutes/minutes_download.go#L122-L123

Added lines #L122 - L123 were not covered by tests
}
}

useDefaultLayout := explicitOutputPath == "" && explicitOutputDir == ""

if !single {
fmt.Fprintf(errOut, "[minutes +download] batch: %d token(s)\n", len(tokens))
}

type result struct {
MinuteToken string `json:"minute_token"`
SavedPath string `json:"saved_path,omitempty"`
SizeBytes int64 `json:"size_bytes,omitempty"`
DownloadURL string `json:"download_url,omitempty"`
Error string `json:"error,omitempty"`
MinuteToken string `json:"minute_token"`
ArtifactType string `json:"artifact_type,omitempty"`
SavedPath string `json:"saved_path,omitempty"`
SizeBytes int64 `json:"size_bytes,omitempty"`
DownloadURL string `json:"download_url,omitempty"`
Error string `json:"error,omitempty"`
}

results := make([]result, len(tokens))
Expand Down Expand Up @@ -160,20 +204,31 @@

fmt.Fprintf(errOut, "Downloading media: %s\n", common.MaskToken(token))

// single token: --output is a file path; batch: --output is a directory
opts := downloadOpts{fio: runtime.FileIO(), overwrite: overwrite, usedNames: usedNames}
if single {
opts.outputPath = outputPath
} else {
opts.outputDir = outputPath
opts := downloadOpts{fio: runtime.FileIO(), overwrite: overwrite}
switch {
case useDefaultLayout:
// Per-token subdirectory guarantees unique paths, so no dedup map.
opts.outputDir = common.DefaultMinuteArtifactDir(token)
case explicitOutputPath != "" && single:
opts.outputPath = explicitOutputPath
default:
opts.outputDir = explicitOutputDir
if !single {
opts.usedNames = usedNames
}
}
Comment thread
coderabbitai[bot] marked this conversation as resolved.

dl, err := downloadMediaFile(ctx, dlClient, downloadURL, token, opts)
if err != nil {
results[i] = result{MinuteToken: token, Error: err.Error()}
continue
}
results[i] = result{MinuteToken: token, SavedPath: dl.savedPath, SizeBytes: dl.sizeBytes}
results[i] = result{
MinuteToken: token,
ArtifactType: common.ArtifactTypeRecording,
SavedPath: dl.savedPath,
SizeBytes: dl.sizeBytes,
}
}

// output
Expand All @@ -183,9 +238,17 @@
return output.ErrAPI(0, r.Error, nil)
}
if urlOnly {
runtime.Out(map[string]interface{}{"download_url": r.DownloadURL}, nil)
runtime.Out(map[string]interface{}{
"minute_token": r.MinuteToken,
"download_url": r.DownloadURL,
}, nil)
} else {
runtime.Out(map[string]interface{}{"saved_path": r.SavedPath, "size_bytes": r.SizeBytes}, nil)
runtime.Out(map[string]interface{}{
"minute_token": r.MinuteToken,
"artifact_type": r.ArtifactType,
"saved_path": r.SavedPath,
"size_bytes": r.SizeBytes,
}, nil)
}
return nil
}
Expand Down Expand Up @@ -230,7 +293,7 @@
type downloadOpts struct {
fio fileio.FileIO // file I/O abstraction
outputPath string // explicit output file path (single mode only)
outputDir string // output directory (batch mode)
outputDir string // output directory (single or batch)
overwrite bool
usedNames map[string]bool // tracks used filenames to deduplicate in batch mode
}
Expand Down Expand Up @@ -300,7 +363,7 @@
func resolveFilenameFromResponse(resp *http.Response, minuteToken string) string {
if cd := resp.Header.Get("Content-Disposition"); cd != "" {
if _, params, err := mime.ParseMediaType(cd); err == nil {
if filename := params["filename"]; filename != "" {
if filename := sanitizeServerFilename(params["filename"]); filename != "" {
return filename
}
}
Expand All @@ -311,6 +374,20 @@
return minuteToken + ".media"
}

// sanitizeServerFilename reduces a server-provided filename to its basename,
// defending against Content-Disposition payloads that embed directory
// separators (e.g. "../other.mp4") and would otherwise escape the intended
// artifact directory after filepath.Join. Empty or dot-only names return ""
// so the caller can fall back to the next naming strategy.
func sanitizeServerFilename(filename string) string {
filename = strings.ReplaceAll(filename, "\\", "/")
filename = path.Base(filename)
if filename == "" || filename == "." || filename == ".." {
return ""
}
return filename
}

// preferredExt overrides Go's mime.ExtensionsByType which returns alphabetically sorted
// results (e.g. .m4v before .mp4 for video/mp4).
var preferredExt = map[string]string{
Expand Down
Loading
Loading