diff --git a/src/clis/nvcf-cli/USAGE-GUIDE.md b/src/clis/nvcf-cli/USAGE-GUIDE.md index 8f4189ab83..b6752e1510 100644 --- a/src/clis/nvcf-cli/USAGE-GUIDE.md +++ b/src/clis/nvcf-cli/USAGE-GUIDE.md @@ -719,6 +719,12 @@ Registry credentials allow NVCF to pull container images from private registries --registry "docker.io" \ --username "myusername" \ --password "mypassword" + +# Add registry credentials securely using a secret file (or '-' for standard input) +./nvcf-cli registry-credential add \ + --hostname "docker.io" \ + --secret-file "/path/to/secret.b64" \ + --artifact-type CONTAINER ``` #### Method C: Using curl directly diff --git a/src/clis/nvcf-cli/cmd/registry.go b/src/clis/nvcf-cli/cmd/registry.go index 5cab0ae6ef..68f2329171 100644 --- a/src/clis/nvcf-cli/cmd/registry.go +++ b/src/clis/nvcf-cli/cmd/registry.go @@ -21,6 +21,8 @@ import ( "context" "encoding/base64" "fmt" + "io" + "os" "strings" "time" @@ -98,9 +100,10 @@ var registryAddCmd = &cobra.Command{ Short: "Add a new registry credential", Long: `Add a new registry credential to access private registries. -You can provide credentials in two ways: +You can provide credentials in three ways: 1. Separate username and password (CLI will encode them automatically) -2. Pre-encoded base64 secret in 'username:password' format +2. Pre-encoded base64 secret in 'username:password' format via --secret +3. Pre-encoded base64 secret from a file or standard input via --secret-file (recommended to avoid exposing secrets in process arguments) You must specify at least one artifact type that this credential can access. @@ -116,6 +119,9 @@ Authentication (choose one): Option 2 - Pre-encoded secret: --secret Base64 encoded 'username:password' string + Option 3 - Secret from file or stdin (recommended): + --secret-file File containing base64 encoded 'username:password' string, or '-' for standard input + Optional flags: --description Description of the credential --tag Tags for the credential - can be specified multiple times @@ -129,7 +135,21 @@ Examples: --artifact-type CONTAINER \ --description "My private Docker registry" - # Add credentials using pre-encoded secret + # Add credentials using a secret file (recommended for security) + nvcf-cli registry-credential add \ + --hostname nvcr.io \ + --secret-file /path/to/secret.b64 \ + --artifact-type CONTAINER \ + --description "NVIDIA Container Registry" + + # Add credentials via standard input + echo -n "${ACCESS_KEY_ID}:${SECRET_ACCESS_KEY}" | base64 | nvcf-cli registry-credential add \ + --hostname public.ecr.aws \ + --secret-file - \ + --artifact-type CONTAINER \ + --description "ECR Public Registry" + + # Add credentials using pre-encoded secret flag nvcf-cli registry-credential add \ --hostname nvcr.io \ --secret "JG9hdXRodG9rZW46ZjRvYm5lbjVrcGhpamZvcTI5NHFnY3Rna3Y6YmQ4YWM0OTEtZDllMi00YWJiLWJmOTQtMTNhMjk2ZTgxYzUw" \ @@ -139,8 +159,7 @@ Examples: # Add credentials for multiple artifact types with tags nvcf-cli registry-credential add \ --hostname myregistry.example.com \ - --username myuser \ - --password mypass \ + --secret-file /path/to/secret.b64 \ --artifact-type CONTAINER \ --artifact-type MODEL \ --tag "environment:prod" \ @@ -241,6 +260,7 @@ func init() { registryAddCmd.Flags().String("username", "", "Registry username (use with --password)") registryAddCmd.Flags().String("password", "", "Registry password (use with --username)") registryAddCmd.Flags().String("secret", "", "Base64 encoded 'username:password' string (alternative to --username/--password)") + registryAddCmd.Flags().String("secret-file", "", "File containing base64 encoded 'username:password' string, or '-' for standard input") registryAddCmd.Flags().StringSlice("artifact-type", []string{}, "Artifact types (CONTAINER, HELM, MODEL, RESOURCE) - required, can be specified multiple times") registryAddCmd.Flags().String("description", "", "Description of the credential") registryAddCmd.Flags().StringSlice("tag", []string{}, "Tags for the credential - can be specified multiple times") @@ -362,6 +382,7 @@ func runListRegistryCredentials(cmd *cobra.Command, args []string) error { return nil } +// runAddRegistryCredential executes the 'registry-credential add' subcommand. func runAddRegistryCredential(cmd *cobra.Command, args []string) error { // Load configuration config, err := client.LoadConfig() @@ -380,16 +401,23 @@ func runAddRegistryCredential(cmd *cobra.Command, args []string) error { username, _ := cmd.Flags().GetString("username") password, _ := cmd.Flags().GetString("password") secret, _ := cmd.Flags().GetString("secret") + secretFile, _ := cmd.Flags().GetString("secret-file") artifactTypeStrs, _ := cmd.Flags().GetStringSlice("artifact-type") description, _ := cmd.Flags().GetString("description") tags, _ := cmd.Flags().GetStringSlice("tag") - encodedCredentials, err := validateAndEncodeCredentials(secret, username, password) + encodedCredentials, err := resolveAndValidateCredentials(secret, secretFile, username, password, cmd.InOrStdin()) if err != nil { return err } if config.Debug { - if secret != "" { + if secretFile != "" { + if secretFile == "-" { + logging.Debug("Using secret from standard input (length: %d chars)", len(encodedCredentials)) + } else { + logging.Debug("Using secret from file '%s' (length: %d chars)", secretFile, len(encodedCredentials)) + } + } else if secret != "" { logging.Debug("Using pre-encoded secret (length: %d chars)", len(secret)) } else { logging.Debug("Encoding username:password to base64 (length: %d chars)", len(encodedCredentials)) @@ -699,26 +727,113 @@ func runListRecognizedRegistries(cmd *cobra.Command, args []string) error { // Helper functions -// validateAndEncodeCredentials validates the mutually-exclusive auth flag combinations +// trimTrailingLineEnding removes only a single trailing line ending (\r\n, \n, or \r) +// from the string, preserving any other whitespace. +func trimTrailingLineEnding(s string) string { + if strings.HasSuffix(s, "\r\n") { + return strings.TrimSuffix(s, "\r\n") + } + if strings.HasSuffix(s, "\n") { + return strings.TrimSuffix(s, "\n") + } + if strings.HasSuffix(s, "\r") { + return strings.TrimSuffix(s, "\r") + } + return s +} + +// validateBase64Secret verifies that the secret is non-empty and decodes as valid base64. +// Both standard padded base64 and unpadded base64 are accepted. +func validateBase64Secret(secret string) error { + if secret == "" { + return fmt.Errorf("secret cannot be empty") + } + if _, err := base64.StdEncoding.DecodeString(secret); err == nil { + return nil + } + if _, err := base64.RawStdEncoding.DecodeString(secret); err == nil { + return nil + } + return fmt.Errorf("invalid base64 credential: must be valid base64 encoded string") +} + +// resolveAndValidateCredentials validates the mutually-exclusive auth flag combinations // for the add command and returns the base64-encoded `username:password` value to send // to the NVCF API. Callers must choose exactly one authentication method: // -// - --secret with a pre-encoded base64 string, OR +// - --secret with an inline pre-encoded base64 string, OR +// - --secret-file with a file path or '-' for standard input, OR // - both --username and --password (this function encodes them). -func validateAndEncodeCredentials(secret, username, password string) (string, error) { - if secret != "" { - if username != "" || password != "" { - return "", fmt.Errorf("cannot use --secret with --username/--password. Choose one authentication method") +func resolveAndValidateCredentials(secret, secretFile, username, password string, stdin io.Reader) (string, error) { + hasSecret := secret != "" + hasSecretFile := secretFile != "" + hasUserOrPass := username != "" || password != "" + + // Check conflicting authentication flags + if hasSecretFile && hasSecret { + return "", fmt.Errorf("cannot use both --secret and --secret-file. Choose one authentication method") + } + if hasSecretFile && hasUserOrPass { + return "", fmt.Errorf("cannot use --secret-file with --username/--password. Choose one authentication method") + } + if hasSecret && hasUserOrPass { + return "", fmt.Errorf("cannot use --secret with --username/--password. Choose one authentication method") + } + + if !hasSecret && !hasSecretFile && !hasUserOrPass { + return "", fmt.Errorf("must provide either --secret, --secret-file, OR both --username and --password") + } + + if hasSecret { + if err := validateBase64Secret(secret); err != nil { + return "", err } return secret, nil } + + if hasSecretFile { + var raw []byte + if secretFile == "-" { + if stdin == nil { + stdin = os.Stdin + } + var err error + raw, err = io.ReadAll(stdin) + if err != nil { + return "", fmt.Errorf("failed to read secret from stdin: %w", err) + } + } else { + var err error + raw, err = os.ReadFile(secretFile) + if err != nil { + return "", fmt.Errorf("failed to read secret file '%s': %w", secretFile, err) + } + } + + trimmed := trimTrailingLineEnding(string(raw)) + if err := validateBase64Secret(trimmed); err != nil { + if secretFile == "-" { + return "", fmt.Errorf("invalid secret from stdin: %w", err) + } + return "", fmt.Errorf("invalid secret from file '%s': %w", secretFile, err) + } + return trimmed, nil + } + if username == "" || password == "" { - return "", fmt.Errorf("must provide either --secret OR both --username and --password") + return "", fmt.Errorf("must provide either --secret, --secret-file, OR both --username and --password") } + credentials := username + ":" + password return base64.StdEncoding.EncodeToString([]byte(credentials)), nil } +// validateAndEncodeCredentials validates authentication flags and returns the base64-encoded +// credential. It is maintained for backwards compatibility with tests and callers that pass inline flags. +func validateAndEncodeCredentials(secret, username, password string) (string, error) { + return resolveAndValidateCredentials(secret, "", username, password, nil) +} + // parseAndValidateArtifactTypes converts user-supplied artifact type strings into // typed values. Comparison is case-insensitive; unknown values produce an error // listing the valid set. diff --git a/src/clis/nvcf-cli/cmd/registry_test.go b/src/clis/nvcf-cli/cmd/registry_test.go index efb779bfe3..2f188e1e7d 100644 --- a/src/clis/nvcf-cli/cmd/registry_test.go +++ b/src/clis/nvcf-cli/cmd/registry_test.go @@ -19,6 +19,8 @@ package cmd import ( "encoding/base64" + "os" + "path/filepath" "strings" "testing" @@ -33,6 +35,7 @@ import ( // command group and its subcommands (NVCF-10082 renamed `registry` -> // `registry-credential`). +// TestRegistryCredentialCommandStructure verifies the registry-credential command structure, subcommands, and flags. func TestRegistryCredentialCommandStructure(t *testing.T) { t.Run("top-level command uses registry-credential", func(t *testing.T) { assert.Equal(t, "registry-credential", registryCmd.Use) @@ -66,7 +69,7 @@ func TestRegistryCredentialCommandStructure(t *testing.T) { }) t.Run("add command exposes secret/username/password flags", func(t *testing.T) { - for _, name := range []string{"hostname", "username", "password", "secret", "artifact-type", "description", "tag"} { + for _, name := range []string{"hostname", "username", "password", "secret", "secret-file", "artifact-type", "description", "tag"} { assert.NotNilf(t, registryAddCmd.Flag(name), "expected --%s flag on add command", name) } }) @@ -84,6 +87,7 @@ func TestRegistryCredentialCommandStructure(t *testing.T) { // --- validateAndEncodeCredentials --- +// TestValidateAndEncodeCredentials tests credential flag combinations and base64 encoding. func TestValidateAndEncodeCredentials(t *testing.T) { t.Run("returns secret as-is when only --secret provided", func(t *testing.T) { got, err := validateAndEncodeCredentials("preencoded==", "", "") @@ -115,19 +119,19 @@ func TestValidateAndEncodeCredentials(t *testing.T) { t.Run("rejects when no auth flags provided", func(t *testing.T) { _, err := validateAndEncodeCredentials("", "", "") require.Error(t, err) - assert.Contains(t, err.Error(), "must provide either --secret OR both --username and --password") + assert.Contains(t, err.Error(), "must provide either --secret, --secret-file, OR both --username and --password") }) t.Run("rejects when only --username provided", func(t *testing.T) { _, err := validateAndEncodeCredentials("", "alice", "") require.Error(t, err) - assert.Contains(t, err.Error(), "must provide either --secret OR both --username and --password") + assert.Contains(t, err.Error(), "must provide either --secret, --secret-file, OR both --username and --password") }) t.Run("rejects when only --password provided", func(t *testing.T) { _, err := validateAndEncodeCredentials("", "", "s3cret") require.Error(t, err) - assert.Contains(t, err.Error(), "must provide either --secret OR both --username and --password") + assert.Contains(t, err.Error(), "must provide either --secret, --secret-file, OR both --username and --password") }) t.Run("encodes passwords containing colons correctly", func(t *testing.T) { @@ -141,6 +145,207 @@ func TestValidateAndEncodeCredentials(t *testing.T) { require.NoError(t, decodeErr) assert.Equal(t, "alice:p:a:s:s", string(decoded)) }) + + t.Run("rejects invalid base64 in --secret", func(t *testing.T) { + _, err := validateAndEncodeCredentials("not-valid-base64!@#$", "", "") + require.Error(t, err) + assert.Contains(t, err.Error(), "invalid base64 credential") + }) +} + +// --- trimTrailingLineEnding --- + +// TestTrimTrailingLineEnding verifies trailing line ending removal behavior for files and stdin. +func TestTrimTrailingLineEnding(t *testing.T) { + t.Run("trims single LF line ending", func(t *testing.T) { + assert.Equal(t, "secret", trimTrailingLineEnding("secret\n")) + }) + + t.Run("trims single CRLF line ending", func(t *testing.T) { + assert.Equal(t, "secret", trimTrailingLineEnding("secret\r\n")) + }) + + t.Run("trims single CR line ending", func(t *testing.T) { + assert.Equal(t, "secret", trimTrailingLineEnding("secret\r")) + }) + + t.Run("returns unchanged when no trailing line ending", func(t *testing.T) { + assert.Equal(t, "secret", trimTrailingLineEnding("secret")) + }) + + t.Run("preserves leading whitespace", func(t *testing.T) { + assert.Equal(t, " secret", trimTrailingLineEnding(" secret\n")) + }) + + t.Run("preserves trailing whitespace before line ending", func(t *testing.T) { + assert.Equal(t, "secret ", trimTrailingLineEnding("secret \n")) + }) + + t.Run("trims only the last line ending when multiple exist", func(t *testing.T) { + assert.Equal(t, "secret\n", trimTrailingLineEnding("secret\n\n")) + }) + + t.Run("returns empty string unchanged", func(t *testing.T) { + assert.Equal(t, "", trimTrailingLineEnding("")) + }) +} + +// --- validateBase64Secret --- + +// TestValidateBase64Secret verifies base64 encoding validation. +func TestValidateBase64Secret(t *testing.T) { + t.Run("accepts standard base64", func(t *testing.T) { + encoded := base64.StdEncoding.EncodeToString([]byte("alice:s3cret")) + assert.NoError(t, validateBase64Secret(encoded)) + }) + + t.Run("accepts raw unpadded base64", func(t *testing.T) { + encoded := base64.RawStdEncoding.EncodeToString([]byte("alice:s3cret")) + assert.NoError(t, validateBase64Secret(encoded)) + }) + + t.Run("rejects empty string", func(t *testing.T) { + err := validateBase64Secret("") + require.Error(t, err) + assert.Contains(t, err.Error(), "secret cannot be empty") + }) + + t.Run("rejects non-base64 content", func(t *testing.T) { + err := validateBase64Secret("this is not base64!") + require.Error(t, err) + assert.Contains(t, err.Error(), "invalid base64 credential") + }) +} + +// --- resolveAndValidateCredentials --- + +// TestResolveAndValidateCredentials tests credential resolution from inline flags, files, and stdin. +func TestResolveAndValidateCredentials(t *testing.T) { + validSecret := base64.StdEncoding.EncodeToString([]byte("user:pass")) + + t.Run("resolves secret from file with trailing newline", func(t *testing.T) { + tmpDir := t.TempDir() + filePath := filepath.Join(tmpDir, "secret.b64") + require.NoError(t, os.WriteFile(filePath, []byte(validSecret+"\n"), 0600)) + + got, err := resolveAndValidateCredentials("", filePath, "", "", nil) + require.NoError(t, err) + assert.Equal(t, validSecret, got) + }) + + t.Run("resolves secret from file with CRLF", func(t *testing.T) { + tmpDir := t.TempDir() + filePath := filepath.Join(tmpDir, "secret.b64") + require.NoError(t, os.WriteFile(filePath, []byte(validSecret+"\r\n"), 0600)) + + got, err := resolveAndValidateCredentials("", filePath, "", "", nil) + require.NoError(t, err) + assert.Equal(t, validSecret, got) + }) + + t.Run("resolves secret from stdin using hyphen", func(t *testing.T) { + stdin := strings.NewReader(validSecret + "\n") + got, err := resolveAndValidateCredentials("", "-", "", "", stdin) + require.NoError(t, err) + assert.Equal(t, validSecret, got) + }) + + t.Run("rejects empty secret file", func(t *testing.T) { + tmpDir := t.TempDir() + filePath := filepath.Join(tmpDir, "empty.b64") + require.NoError(t, os.WriteFile(filePath, []byte(""), 0600)) + + _, err := resolveAndValidateCredentials("", filePath, "", "", nil) + require.Error(t, err) + assert.Contains(t, err.Error(), "secret cannot be empty") + }) + + t.Run("rejects empty stdin", func(t *testing.T) { + stdin := strings.NewReader("") + _, err := resolveAndValidateCredentials("", "-", "", "", stdin) + require.Error(t, err) + assert.Contains(t, err.Error(), "secret cannot be empty") + }) + + t.Run("rejects stdin with only newline", func(t *testing.T) { + stdin := strings.NewReader("\n") + _, err := resolveAndValidateCredentials("", "-", "", "", stdin) + require.Error(t, err) + assert.Contains(t, err.Error(), "secret cannot be empty") + }) + + t.Run("rejects non-existent file", func(t *testing.T) { + _, err := resolveAndValidateCredentials("", "/path/to/nonexistent/secret.b64", "", "", nil) + require.Error(t, err) + assert.Contains(t, err.Error(), "failed to read secret file") + }) + + t.Run("rejects invalid base64 in file", func(t *testing.T) { + tmpDir := t.TempDir() + filePath := filepath.Join(tmpDir, "invalid.b64") + require.NoError(t, os.WriteFile(filePath, []byte("plain-text-credentials\n"), 0600)) + + _, err := resolveAndValidateCredentials("", filePath, "", "", nil) + require.Error(t, err) + assert.Contains(t, err.Error(), "invalid secret from file") + assert.Contains(t, err.Error(), "invalid base64 credential") + }) + + t.Run("rejects invalid base64 in stdin", func(t *testing.T) { + stdin := strings.NewReader("plain-text-credentials\n") + _, err := resolveAndValidateCredentials("", "-", "", "", stdin) + require.Error(t, err) + assert.Contains(t, err.Error(), "invalid secret from stdin") + assert.Contains(t, err.Error(), "invalid base64 credential") + }) + + t.Run("rejects secret-file with leading space that breaks base64", func(t *testing.T) { + tmpDir := t.TempDir() + filePath := filepath.Join(tmpDir, "space.b64") + require.NoError(t, os.WriteFile(filePath, []byte(" "+validSecret+"\n"), 0600)) + + _, err := resolveAndValidateCredentials("", filePath, "", "", nil) + require.Error(t, err) + assert.Contains(t, err.Error(), "invalid secret from file") + }) + + t.Run("rejects --secret combined with --secret-file", func(t *testing.T) { + _, err := resolveAndValidateCredentials(validSecret, "/path/to/file", "", "", nil) + require.Error(t, err) + assert.Contains(t, err.Error(), "cannot use both --secret and --secret-file") + }) + + t.Run("rejects --secret-file combined with --username", func(t *testing.T) { + _, err := resolveAndValidateCredentials("", "/path/to/file", "alice", "", nil) + require.Error(t, err) + assert.Contains(t, err.Error(), "cannot use --secret-file with --username/--password") + }) + + t.Run("rejects --secret-file combined with --password", func(t *testing.T) { + _, err := resolveAndValidateCredentials("", "/path/to/file", "", "s3cret", nil) + require.Error(t, err) + assert.Contains(t, err.Error(), "cannot use --secret-file with --username/--password") + }) + + t.Run("rejects --secret-file combined with both --username and --password", func(t *testing.T) { + _, err := resolveAndValidateCredentials("", "/path/to/file", "alice", "s3cret", nil) + require.Error(t, err) + assert.Contains(t, err.Error(), "cannot use --secret-file with --username/--password") + }) + + t.Run("resolves valid base64 --secret", func(t *testing.T) { + got, err := resolveAndValidateCredentials(validSecret, "", "", "", nil) + require.NoError(t, err) + assert.Equal(t, validSecret, got) + }) + + t.Run("resolves username and password", func(t *testing.T) { + got, err := resolveAndValidateCredentials("", "", "alice", "s3cret", nil) + require.NoError(t, err) + decoded, decodeErr := base64.StdEncoding.DecodeString(got) + require.NoError(t, decodeErr) + assert.Equal(t, "alice:s3cret", string(decoded)) + }) } // --- parseAndValidateArtifactTypes --- @@ -244,6 +449,7 @@ func TestFormatTimestamp(t *testing.T) { // --- Sanity check: top-level Use constant matches help text --- +// TestRegistryCredentialUseMatchesHelp verifies subcommand and top-level long help reference the new command name. func TestRegistryCredentialUseMatchesHelp(t *testing.T) { // Quick sanity that all subcommand long-help references use the new // command name. This catches drift if anyone re-introduces "registry " @@ -261,3 +467,13 @@ func TestRegistryCredentialUseMatchesHelp(t *testing.T) { assert.True(t, strings.Contains(registryCmd.Long, "nvcf-cli registry-credential"), "top-level command long help missing `nvcf-cli registry-credential` reference") } + +// TestRegistryCredentialAddHelp verifies --secret-file documentation and examples in help text. +func TestRegistryCredentialAddHelp(t *testing.T) { + assert.Contains(t, registryAddCmd.Long, "--secret-file", + "registry add command help should describe --secret-file") + assert.Contains(t, registryAddCmd.Long, "--secret-file /path/to/secret.b64", + "registry add command help should provide file example") + assert.Contains(t, registryAddCmd.Long, "--secret-file -", + "registry add command help should provide stdin example") +}