diff --git a/internal/cli/entry_test.go b/internal/cli/entry_test.go index 73c266a..692c9f5 100644 --- a/internal/cli/entry_test.go +++ b/internal/cli/entry_test.go @@ -1664,20 +1664,119 @@ func TestGenerateValidatesInputPaths(t *testing.T) { }) } -// TestFlagAfterArgumentIsArgument asserts that flags are read only before a -// command's first argument: after it, -v is a path, not the verbose flag. -func TestFlagAfterArgumentIsArgument(t *testing.T) { - t.Parallel() +// generateTestDir writes testFile1 and runs gen on testDir, which writes +// the manifest to testDir/index.mf. +func generateTestDir(t *testing.T, fs afero.Fs) { + t.Helper() - fs := afero.NewMemMapFs() require.NoError(t, fs.MkdirAll(testDir, 0o755)) writeTestFile(t, fs, testFile1, "content") - opts := testOpts([]string{testApp, cmdGenerate, testDir, "-v"}, fs) - exitCode := runCLI(opts) + opts := testOpts([]string{testApp, cmdGenerate, "-q", testDir}, fs) + require.Equal(t, 0, runCLI(opts), testStderr(t, opts)) +} - assert.Equal(t, 1, exitCode) - assert.Contains(t, testStderr(t, opts), "path does not exist: -v") +// TestCheckReadsOptionAfterPath runs check on an unsigned manifest with +// --require-signature after the directory. check must refuse the manifest. +func TestCheckReadsOptionAfterPath(t *testing.T) { + t.Parallel() + + fs := afero.NewMemMapFs() + generateTestDir(t, fs) + + opts := testOpts([]string{ + testApp, cmdCheck, "-q", testDir, "--" + flagRequireSignature, msgFpA, + }, fs) + assert.Equal(t, 1, runCLI(opts)) + assert.Contains(t, testStderr(t, opts), "manifest is not signed") +} + +// TestFreshenReadsOptionAfterPath runs freshen with --include-permissions +// after the directory. freshen must record the file's mode. +func TestFreshenReadsOptionAfterPath(t *testing.T) { + t.Parallel() + + fs := afero.NewOsFs() + root, manifestPath := setupFreshenDir(t, fs, + map[string]string{testFileTxt: "a file"}) + require.NoError(t, fs.Chmod(filepath.Join(root, testFileTxt), 0o640)) + + opts := testOpts([]string{ + testApp, cmdFreshen, "-q", root, "--" + flagIncludePermissions, + }, fs) + require.Equal(t, 0, runCLI(opts), testStderr(t, opts)) + + files := manifestFiles(t, fs, manifestPath) + require.Len(t, files, 1) + assert.Equal(t, uint32(0o640), files[0].GetMode()) +} + +// TestFetchReadsOptionAfterURL runs fetch with --dest after the URL. The +// file and the manifest must land in that directory and nothing in the +// current one. +// +//nolint:paralleltest // changes the process-global working directory +func TestFetchReadsOptionAfterURL(t *testing.T) { + content := []byte("fetched") + manifest := manifestOf(t, map[string][]byte{testFileTxt: content}) + + server := httptest.NewServer(fetchTestHandler(manifest, + map[string][]byte{testFileTxt: content})) + defer server.Close() + + cwd := chdirTemp(t) + dest := t.TempDir() + + opts := testOpts([]string{ + testApp, cmdFetch, "-q", server.URL, "--" + flagDest, dest, + }, afero.NewOsFs()) + require.Equal(t, 0, runCLI(opts), testStderr(t, opts)) + + assert.Equal(t, + map[string][]byte{testFileTxt: content, defaultManifestName: manifest}, + filesUnder(t, dest)) + assert.Empty(t, filesUnder(t, cwd), "fetch wrote outside --dest") +} + +// TestOneArgumentCommandsRefuseSecond runs each command that takes one +// argument with two. It must fail naming the second. +func TestOneArgumentCommandsRefuseSecond(t *testing.T) { + t.Parallel() + + for _, command := range []string{ + cmdCheck, cmdFreshen, cmdList, cmdExport, cmdFetch, + } { + t.Run(command, func(t *testing.T) { + t.Parallel() + + fs := afero.NewMemMapFs() + generateTestDir(t, fs) + + opts := testOpts([]string{testApp, command, testDir, "/second"}, fs) + assert.Equal(t, 1, runCLI(opts)) + assert.Contains(t, testStderr(t, opts), + `unexpected argument "/second"`) + }) + } +} + +// TestGenerateReadsPathAfterDoubleDash runs gen -- -x where -x is a +// directory. -x must be read as the path to list, not as an option. +// +//nolint:paralleltest // changes the process-global working directory +func TestGenerateReadsPathAfterDoubleDash(t *testing.T) { + chdirTemp(t) + + fs := afero.NewOsFs() + require.NoError(t, fs.Mkdir("-x", 0o755)) + writeTestFile(t, fs, filepath.Join("-x", testFileTxt), "content") + + opts := testOpts([]string{testApp, cmdGenerate, "-q", "--", "-x"}, fs) + require.Equal(t, 0, runCLI(opts), testStderr(t, opts)) + + files := manifestFiles(t, fs, filepath.Join("-x", defaultManifestName)) + require.Len(t, files, 1) + assert.Equal(t, testFileTxt, files[0].GetPath()) } func TestCheckDetectsManifestCorruption(t *testing.T) { diff --git a/internal/cli/mfer.go b/internal/cli/mfer.go index bd51420..b3fe903 100644 --- a/internal/cli/mfer.go +++ b/internal/cli/mfer.go @@ -48,6 +48,10 @@ func manifestTempPath(out string) string { // errUnknownCommand indicates an unrecognized command argument. var errUnknownCommand = errors.New("unknown command") +// errUnexpectedArgument indicates a second argument to a command that takes +// one. +var errUnexpectedArgument = errors.New("unexpected argument") + // CLIApp is the main CLI application container. It holds configuration, // I/O streams, and filesystem abstraction to enable testing and flexibility. // @@ -154,13 +158,14 @@ func commonFlags() []cli.Flag { } } -// stopOnFirstArg returns the StopOnNthArg setting every command uses: flags -// are read only before the command's first argument, and everything after it -// is an argument, as with urfave/cli v2. So `mfer gen d -v` names a path "-v". -func stopOnFirstArg() *int { - n := 1 +// refuseExtraArgs is the ArgValidator of the commands that take one +// argument: it refuses a second one, naming it. +func refuseExtraArgs(_ context.Context, cmd *cli.Command) error { + if cmd.Args().Len() > 1 { + return fmt.Errorf("%w %q", errUnexpectedArgument, cmd.Args().Get(1)) + } - return &n + return nil } // requireSignatureFlag returns the --require-signature flag taken by the @@ -186,11 +191,10 @@ func includePermissionsFlag() *cli.BoolFlag { func (mfa *CLIApp) generateCommand() *cli.Command { return &cli.Command{ - Name: cmdGenerate, - Aliases: []string{"gen"}, - Usage: "Generate manifest file", - ArgsUsage: "[path ...]", - StopOnNthArg: stopOnFirstArg(), + Name: cmdGenerate, + Aliases: []string{"gen"}, + Usage: "Generate manifest file", + ArgsUsage: "[path ...]", Action: func(ctx context.Context, cmd *cli.Command) error { mfa.setVerbosity(cmd) mfa.printBanner() @@ -252,7 +256,7 @@ func (mfa *CLIApp) checkCommand() *cli.Command { Name: cmdCheck, Usage: "Validate files using manifest file", ArgsUsage: manifestArgsUsage, - StopOnNthArg: stopOnFirstArg(), + ArgValidator: refuseExtraArgs, Action: func(ctx context.Context, cmd *cli.Command) error { mfa.setVerbosity(cmd) mfa.printBanner() @@ -286,7 +290,7 @@ func (mfa *CLIApp) freshenCommand() *cli.Command { Name: cmdFreshen, Usage: "Update manifest with changed, new, and removed files", ArgsUsage: manifestArgsUsage, - StopOnNthArg: stopOnFirstArg(), + ArgValidator: refuseExtraArgs, Action: func(ctx context.Context, cmd *cli.Command) error { mfa.setVerbosity(cmd) mfa.printBanner() @@ -337,7 +341,7 @@ func (mfa *CLIApp) exportCommand() *cli.Command { Name: cmdExport, Usage: "Export manifest contents as JSON", ArgsUsage: "[manifest file or URL]", - StopOnNthArg: stopOnFirstArg(), + ArgValidator: refuseExtraArgs, Action: func(ctx context.Context, cmd *cli.Command) error { mfa.setVerbosity(cmd) @@ -348,9 +352,8 @@ func (mfa *CLIApp) exportCommand() *cli.Command { func (mfa *CLIApp) versionCommand() *cli.Command { return &cli.Command{ - Name: cmdVersion, - Usage: "Show version", - StopOnNthArg: stopOnFirstArg(), + Name: cmdVersion, + Usage: "Show version", Action: func(context.Context, *cli.Command) error { mfa.printVersion() @@ -365,7 +368,7 @@ func (mfa *CLIApp) listCommand() *cli.Command { Aliases: []string{"ls"}, Usage: "List files in manifest", ArgsUsage: manifestArgsUsage, - StopOnNthArg: stopOnFirstArg(), + ArgValidator: refuseExtraArgs, Action: mfa.listManifestOperation, Flags: []cli.Flag{ &cli.BoolFlag{ @@ -386,7 +389,7 @@ func (mfa *CLIApp) fetchCommand() *cli.Command { Name: cmdFetch, Usage: "fetch manifest and referenced files", ArgsUsage: "URL", - StopOnNthArg: stopOnFirstArg(), + ArgValidator: refuseExtraArgs, Action: func(ctx context.Context, cmd *cli.Command) error { mfa.setVerbosity(cmd) mfa.printBanner() @@ -445,9 +448,8 @@ func (mfa *CLIApp) run(args []string) { Writer: mfa.Stdout, // v3 writes its "Incorrect Usage" line to ErrWriter; v2 wrote it to // stdout, before the help, so it stays on stdout. - ErrWriter: mfa.Stdout, - Flags: commonFlags(), - StopOnNthArg: stopOnFirstArg(), + ErrWriter: mfa.Stdout, + Flags: commonFlags(), Action: func(_ context.Context, cmd *cli.Command) error { if cmd.Args().Len() > 0 { return fmt.Errorf("%w %q", errUnknownCommand, cmd.Args().First())