diff --git a/cli/command/config/create.go b/cli/command/config/create.go index ed72e432ba..5c27b270d1 100644 --- a/cli/command/config/create.go +++ b/cli/command/config/create.go @@ -51,17 +51,7 @@ func newConfigCreateCommand(dockerCli command.Cli) *cobra.Command { func RunConfigCreate(ctx context.Context, dockerCLI command.Cli, options CreateOptions) error { apiClient := dockerCLI.Client() - var in io.Reader = dockerCLI.In() - if options.File != "-" { - file, err := sequential.Open(options.File) - if err != nil { - return err - } - in = file - defer file.Close() - } - - configData, err := io.ReadAll(in) + configData, err := readConfigData(dockerCLI.In(), options.File) if err != nil { return errors.Errorf("Error reading content from %q: %v", options.File, err) } @@ -83,6 +73,54 @@ func RunConfigCreate(ctx context.Context, dockerCLI command.Cli, options CreateO return err } - fmt.Fprintln(dockerCLI.Out(), r.ID) + _, _ = fmt.Fprintln(dockerCLI.Out(), r.ID) return nil } + +// maxConfigSize is the maximum byte length of the [swarm.ConfigSpec.Data] field, +// as defined by [MaxConfigSize] in SwarmKit. +// +// [MaxConfigSize]: https://pkg.go.dev/github.com/moby/swarmkit/v2@v2.0.0-20250103191802-8c1959736554/manager/controlapi#MaxConfigSize +const maxConfigSize = 1000 * 1024 // 1000KB + +// readConfigData reads the config from either stdin or the given fileName. +// +// It reads up to twice the maximum size of the config ([maxConfigSize]), +// just in case swarm's limit changes; this is only a safeguard to prevent +// reading arbitrary files into memory. +func readConfigData(in io.Reader, fileName string) ([]byte, error) { + switch fileName { + case "-": + data, err := io.ReadAll(io.LimitReader(in, 2*maxConfigSize)) + if err != nil { + return nil, fmt.Errorf("error reading from STDIN: %w", err) + } + if len(data) == 0 { + return nil, errors.New("error reading from STDIN: data is empty") + } + return data, nil + case "": + return nil, errors.New("config file is required") + default: + // Open file with [FILE_FLAG_SEQUENTIAL_SCAN] on Windows, which + // prevents Windows from aggressively caching it. We expect this + // file to be only read once. Given that this is expected to be + // a small file, this may not be a significant optimization, so + // we could choose to omit this, and use a regular [os.Open]. + // + // [FILE_FLAG_SEQUENTIAL_SCAN]: https://learn.microsoft.com/en-us/windows/win32/api/fileapi/nf-fileapi-createfilea#FILE_FLAG_SEQUENTIAL_SCAN + f, err := sequential.Open(fileName) + if err != nil { + return nil, fmt.Errorf("error reading from %s: %w", fileName, err) + } + defer f.Close() + data, err := io.ReadAll(io.LimitReader(f, 2*maxConfigSize)) + if err != nil { + return nil, fmt.Errorf("error reading from %s: %w", fileName, err) + } + if len(data) == 0 { + return nil, fmt.Errorf("error reading from %s: data is empty", fileName) + } + return data, nil + } +} diff --git a/cli/command/config/create_test.go b/cli/command/config/create_test.go index 9c919f8800..0b2976f027 100644 --- a/cli/command/config/create_test.go +++ b/cli/command/config/create_test.go @@ -59,7 +59,7 @@ func TestConfigCreateErrors(t *testing.T) { } func TestConfigCreateWithName(t *testing.T) { - name := "foo" + const name = "config-with-name" var actual []byte cli := test.NewFakeCli(&fakeClient{ configCreateFunc: func(_ context.Context, spec swarm.ConfigSpec) (types.ConfigCreateResponse, error) { @@ -87,7 +87,7 @@ func TestConfigCreateWithLabels(t *testing.T) { "lbl1": "Label-foo", "lbl2": "Label-bar", } - name := "foo" + const name = "config-with-labels" data, err := os.ReadFile(filepath.Join("testdata", configDataFile)) assert.NilError(t, err) @@ -124,7 +124,7 @@ func TestConfigCreateWithTemplatingDriver(t *testing.T) { expectedDriver := &swarm.Driver{ Name: "template-driver", } - name := "foo" + const name = "config-with-template-driver" cli := test.NewFakeCli(&fakeClient{ configCreateFunc: func(_ context.Context, spec swarm.ConfigSpec) (types.ConfigCreateResponse, error) { diff --git a/cli/command/secret/create.go b/cli/command/secret/create.go index 706e92c3d1..dd8a834135 100644 --- a/cli/command/secret/create.go +++ b/cli/command/secret/create.go @@ -52,14 +52,19 @@ func newSecretCreateCommand(dockerCli command.Cli) *cobra.Command { func runSecretCreate(ctx context.Context, dockerCli command.Cli, options createOptions) error { client := dockerCli.Client() - if options.driver != "" && options.file != "" { - return errors.Errorf("When using secret driver secret data must be empty") + var secretData []byte + if options.driver != "" { + if options.file != "" { + return errors.Errorf("When using secret driver secret data must be empty") + } + } else { + var err error + secretData, err = readSecretData(dockerCli.In(), options.file) + if err != nil { + return err + } } - secretData, err := readSecretData(dockerCli.In(), options.file) - if err != nil { - return errors.Errorf("Error reading content from %q: %v", options.file, err) - } spec := swarm.SecretSpec{ Annotations: swarm.Annotations{ Name: options.name, @@ -82,26 +87,54 @@ func runSecretCreate(ctx context.Context, dockerCli command.Cli, options createO return err } - fmt.Fprintln(dockerCli.Out(), r.ID) + _, _ = fmt.Fprintln(dockerCli.Out(), r.ID) return nil } -func readSecretData(in io.ReadCloser, file string) ([]byte, error) { - // Read secret value from external driver - if file == "" { - return nil, nil - } - if file != "-" { - var err error - in, err = sequential.Open(file) +// maxSecretSize is the maximum byte length of the [swarm.SecretSpec.Data] field, +// as defined by [MaxSecretSize] in SwarmKit. +// +// [MaxSecretSize]: https://pkg.go.dev/github.com/moby/swarmkit/v2@v2.0.0-20250103191802-8c1959736554/api/validation#MaxSecretSize +const maxSecretSize = 500 * 1024 // 500KB + +// readSecretData reads the secret from either stdin or the given fileName. +// +// It reads up to twice the maximum size of the secret ([maxSecretSize]), +// just in case swarm's limit changes; this is only a safeguard to prevent +// reading arbitrary files into memory. +func readSecretData(in io.Reader, fileName string) ([]byte, error) { + switch fileName { + case "-": + data, err := io.ReadAll(io.LimitReader(in, 2*maxSecretSize)) if err != nil { - return nil, err + return nil, fmt.Errorf("error reading from STDIN: %w", err) } - defer in.Close() + if len(data) == 0 { + return nil, errors.New("error reading from STDIN: data is empty") + } + return data, nil + case "": + return nil, errors.New("secret file is required") + default: + // Open file with [FILE_FLAG_SEQUENTIAL_SCAN] on Windows, which + // prevents Windows from aggressively caching it. We expect this + // file to be only read once. Given that this is expected to be + // a small file, this may not be a significant optimization, so + // we could choose to omit this, and use a regular [os.Open]. + // + // [FILE_FLAG_SEQUENTIAL_SCAN]: https://learn.microsoft.com/en-us/windows/win32/api/fileapi/nf-fileapi-createfilea#FILE_FLAG_SEQUENTIAL_SCAN + f, err := sequential.Open(fileName) + if err != nil { + return nil, fmt.Errorf("error reading from %s: %w", fileName, err) + } + defer f.Close() + data, err := io.ReadAll(io.LimitReader(f, 2*maxSecretSize)) + if err != nil { + return nil, fmt.Errorf("error reading from %s: %w", fileName, err) + } + if len(data) == 0 { + return nil, fmt.Errorf("error reading from %s: data is empty", fileName) + } + return data, nil } - data, err := io.ReadAll(in) - if err != nil { - return nil, err - } - return data, nil } diff --git a/cli/command/secret/create_test.go b/cli/command/secret/create_test.go index 884b7d771d..17520728c4 100644 --- a/cli/command/secret/create_test.go +++ b/cli/command/secret/create_test.go @@ -56,7 +56,7 @@ func TestSecretCreateErrors(t *testing.T) { } func TestSecretCreateWithName(t *testing.T) { - name := "foo" + const name = "secret-with-name" data, err := os.ReadFile(filepath.Join("testdata", secretDataFile)) assert.NilError(t, err) @@ -89,7 +89,7 @@ func TestSecretCreateWithDriver(t *testing.T) { expectedDriver := &swarm.Driver{ Name: "secret-driver", } - name := "foo" + const name = "secret-with-driver" cli := test.NewFakeCli(&fakeClient{ secretCreateFunc: func(_ context.Context, spec swarm.SecretSpec) (types.SecretCreateResponse, error) { @@ -118,7 +118,7 @@ func TestSecretCreateWithTemplatingDriver(t *testing.T) { expectedDriver := &swarm.Driver{ Name: "template-driver", } - const name = "foo" + const name = "secret-with-template-driver" cli := test.NewFakeCli(&fakeClient{ secretCreateFunc: func(_ context.Context, spec swarm.SecretSpec) (types.SecretCreateResponse, error) { @@ -137,7 +137,7 @@ func TestSecretCreateWithTemplatingDriver(t *testing.T) { }) cmd := newSecretCreateCommand(cli) - cmd.SetArgs([]string{name}) + cmd.SetArgs([]string{name, filepath.Join("testdata", secretDataFile)}) assert.Check(t, cmd.Flags().Set("template-driver", expectedDriver.Name)) assert.NilError(t, cmd.Execute()) assert.Check(t, is.Equal("ID-"+name, strings.TrimSpace(cli.OutBuffer().String()))) @@ -148,7 +148,7 @@ func TestSecretCreateWithLabels(t *testing.T) { "lbl1": "Label-foo", "lbl2": "Label-bar", } - const name = "foo" + const name = "secret-with-labels" cli := test.NewFakeCli(&fakeClient{ secretCreateFunc: func(_ context.Context, spec swarm.SecretSpec) (types.SecretCreateResponse, error) {