diff --git a/core/commands/add.go b/core/commands/add.go index 40702921c50..f314bbf648a 100644 --- a/core/commands/add.go +++ b/core/commands/add.go @@ -11,6 +11,7 @@ import ( "github.com/ipfs/kubo/config" "github.com/ipfs/kubo/core/commands/cmdenv" + "github.com/ipfs/kubo/core/commands/cmdutils" "github.com/cheggaaa/pb" "github.com/ipfs/boxo/files" @@ -269,6 +270,13 @@ https://github.com/ipfs/kubo/blob/master/docs/config.md#import return fmt.Errorf("inline-limit %d exceeds maximum allowed size of %d bytes", inlineLimit, verifcid.DefaultMaxIdentityDigestSize) } + // Validate pin name + if pinNameSet { + if err := cmdutils.ValidatePinName(pinName); err != nil { + return err + } + } + toFilesStr, toFilesSet := req.Options[toFilesOptionName].(string) preserveMode, _ := req.Options[preserveModeOptionName].(bool) preserveMtime, _ := req.Options[preserveMtimeOptionName].(bool) diff --git a/core/commands/cmdutils/utils.go b/core/commands/cmdutils/utils.go index be295f9e314..9ecfd1446d8 100644 --- a/core/commands/cmdutils/utils.go +++ b/core/commands/cmdutils/utils.go @@ -13,6 +13,7 @@ import ( const ( AllowBigBlockOptionName = "allow-big-block" SoftBlockLimit = 1024 * 1024 // https://github.com/ipfs/kubo/issues/7421#issuecomment-910833499 + MaxPinNameBytes = 255 // Maximum number of bytes allowed for a pin name ) var AllowBigBlockOption cmds.Option @@ -50,6 +51,21 @@ func CheckBlockSize(req *cmds.Request, size uint64) error { return nil } +// ValidatePinName validates that a pin name does not exceed the maximum allowed byte length. +// Returns an error if the name exceeds MaxPinNameBytes (255 bytes). +func ValidatePinName(name string) error { + if name == "" { + // Empty names are allowed + return nil + } + + nameBytes := len([]byte(name)) + if nameBytes > MaxPinNameBytes { + return fmt.Errorf("pin name is %d bytes (max %d bytes)", nameBytes, MaxPinNameBytes) + } + return nil +} + // PathOrCidPath returns a path.Path built from the argument. It keeps the old // behaviour by building a path from a CID string. func PathOrCidPath(str string) (path.Path, error) { diff --git a/core/commands/pin/pin.go b/core/commands/pin/pin.go index 86fafa539a3..0934489d2cb 100644 --- a/core/commands/pin/pin.go +++ b/core/commands/pin/pin.go @@ -100,6 +100,11 @@ It may take some time. Pass '--progress' to track the progress. name, _ := req.Options[pinNameOptionName].(string) showProgress, _ := req.Options[pinProgressOptionName].(bool) + // Validate pin name + if err := cmdutils.ValidatePinName(name); err != nil { + return err + } + if err := req.ParseBodyArgs(); err != nil { return err } @@ -385,6 +390,11 @@ Example: displayNames, _ := req.Options[pinNamesOptionName].(bool) name, _ := req.Options[pinNameOptionName].(string) + // Validate name filter + if err := cmdutils.ValidatePinName(name); err != nil { + return err + } + mode, ok := pin.StringToMode(typeStr) if !ok { return fmt.Errorf("invalid type '%s', must be one of {direct, indirect, recursive, all}", typeStr) diff --git a/core/commands/pin/remotepin.go b/core/commands/pin/remotepin.go index 068d15d0bd8..3936ce635df 100644 --- a/core/commands/pin/remotepin.go +++ b/core/commands/pin/remotepin.go @@ -171,6 +171,10 @@ NOTE: a comma-separated notation is supported in CLI for convenience: opts := []pinclient.AddOption{} if name, nameFound := req.Options[pinNameOptionName]; nameFound { nameStr := name.(string) + // Validate pin name + if err := cmdutils.ValidatePinName(nameStr); err != nil { + return err + } opts = append(opts, pinclient.PinOpts.WithName(nameStr)) } @@ -321,6 +325,11 @@ func lsRemote(ctx context.Context, req *cmds.Request, c *pinclient.Client, out c opts := []pinclient.LsOption{} if name, nameFound := req.Options[pinNameOptionName]; nameFound { nameStr := name.(string) + // Validate name filter + if err := cmdutils.ValidatePinName(nameStr); err != nil { + close(out) + return err + } opts = append(opts, pinclient.PinOpts.FilterName(nameStr)) } diff --git a/docs/changelogs/v0.38.md b/docs/changelogs/v0.38.md index 045715e680a..7f50eab9b5b 100644 --- a/docs/changelogs/v0.38.md +++ b/docs/changelogs/v0.38.md @@ -15,6 +15,7 @@ This release was brought to you by the [Shipyard](https://ipshipyard.com/) team. - [📊 Exposed DHT metrics](#-exposed-dht-metrics) - [🚨 Improved gateway error pages with diagnostic tools](#-improved-gateway-error-pages-with-diagnostic-tools) - [🎨 Updated WebUI](#-updated-webui) + - [📌 Pin name improvements](#-pin-name-improvements) - [🛠️ Identity CID size enforcement and `ipfs files write` fixes](#️-identity-cid-size-enforcement-and-ipfs-files-write-fixes) - [📦️ Important dependency updates](#-important-dependency-updates) - [📝 Changelog](#-changelog) @@ -91,7 +92,7 @@ Additional improvements include a close button in the file viewer, better error #### 📌 Pin name improvements -`ipfs pin ls --names` now correctly returns pin names for specific CIDs ([#10649](https://github.com/ipfs/kubo/issues/10649), [boxo#1035](https://github.com/ipfs/boxo/pull/1035)), and RPC no longer incorrectly returns names from other pins ([#10966](https://github.com/ipfs/kubo/pull/10966)). +`ipfs pin ls --names` now correctly returns pin names for specific CIDs ([#10649](https://github.com/ipfs/kubo/issues/10649), [boxo#1035](https://github.com/ipfs/boxo/pull/1035)), RPC no longer incorrectly returns names from other pins ([#10966](https://github.com/ipfs/kubo/pull/10966)), and pin names are now limited to 255 bytes for better cross-platform compatibility ([#10981](https://github.com/ipfs/kubo/pull/10981)). #### 🛠️ Identity CID size enforcement and `ipfs files write` fixes diff --git a/test/cli/pin_name_validation_test.go b/test/cli/pin_name_validation_test.go new file mode 100644 index 00000000000..049118642e1 --- /dev/null +++ b/test/cli/pin_name_validation_test.go @@ -0,0 +1,184 @@ +package cli + +import ( + "fmt" + "strings" + "testing" + + "github.com/ipfs/kubo/test/cli/harness" + "github.com/stretchr/testify/require" +) + +func TestPinNameValidation(t *testing.T) { + t.Parallel() + + // Create a test node and add a test file + node := harness.NewT(t).NewNode().Init().StartDaemon("--offline") + defer node.StopDaemon() + + // Add a test file to get a CID + testContent := "test content for pin name validation" + testCID := node.IPFSAddStr(testContent, "--pin=false") + + t.Run("pin add accepts valid names", func(t *testing.T) { + testCases := []struct { + name string + pinName string + description string + }{ + { + name: "empty_name", + pinName: "", + description: "Empty name should be allowed", + }, + { + name: "short_name", + pinName: "test", + description: "Short ASCII name should be allowed", + }, + { + name: "max_255_bytes", + pinName: strings.Repeat("a", 255), + description: "Exactly 255 bytes should be allowed", + }, + { + name: "unicode_within_limit", + pinName: "测试名称🔥", // Chinese characters and emoji + description: "Unicode characters within 255 bytes should be allowed", + }, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + var args []string + if tc.pinName != "" { + args = []string{"pin", "add", "--name", tc.pinName, testCID} + } else { + args = []string{"pin", "add", testCID} + } + + res := node.RunIPFS(args...) + require.Equal(t, 0, res.ExitCode(), tc.description) + + // Clean up - unpin + node.RunIPFS("pin", "rm", testCID) + }) + } + }) + + t.Run("pin add rejects names exceeding 255 bytes", func(t *testing.T) { + testCases := []struct { + name string + pinName string + description string + }{ + { + name: "256_bytes", + pinName: strings.Repeat("a", 256), + description: "256 bytes should be rejected", + }, + { + name: "300_bytes", + pinName: strings.Repeat("b", 300), + description: "300 bytes should be rejected", + }, + { + name: "unicode_exceeding_limit", + pinName: strings.Repeat("测", 100), // Each Chinese character is 3 bytes, total 300 bytes + description: "Unicode string exceeding 255 bytes should be rejected", + }, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + res := node.RunIPFS("pin", "add", "--name", tc.pinName, testCID) + require.NotEqual(t, 0, res.ExitCode(), tc.description) + require.Contains(t, res.Stderr.String(), "max 255 bytes", "Error should mention the 255 byte limit") + }) + } + }) + + t.Run("pin ls with name filter validates length", func(t *testing.T) { + // Test valid filter + res := node.RunIPFS("pin", "ls", "--name", strings.Repeat("a", 255)) + require.Equal(t, 0, res.ExitCode(), "255-byte name filter should be accepted") + + // Test invalid filter + res = node.RunIPFS("pin", "ls", "--name", strings.Repeat("a", 256)) + require.NotEqual(t, 0, res.ExitCode(), "256-byte name filter should be rejected") + require.Contains(t, res.Stderr.String(), "max 255 bytes", "Error should mention the 255 byte limit") + }) +} + +func TestAddPinNameValidation(t *testing.T) { + t.Parallel() + + node := harness.NewT(t).NewNode().Init().StartDaemon("--offline") + defer node.StopDaemon() + + // Create a test file + testFile := "test.txt" + node.WriteBytes(testFile, []byte("test content for add command")) + + t.Run("ipfs add with --pin-name accepts valid names", func(t *testing.T) { + testCases := []struct { + name string + pinName string + description string + }{ + { + name: "short_name", + pinName: "test-add", + description: "Short ASCII name should be allowed", + }, + { + name: "max_255_bytes", + pinName: strings.Repeat("x", 255), + description: "Exactly 255 bytes should be allowed", + }, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + res := node.RunIPFS("add", fmt.Sprintf("--pin-name=%s", tc.pinName), "-q", testFile) + require.Equal(t, 0, res.ExitCode(), tc.description) + cid := strings.TrimSpace(res.Stdout.String()) + + // Verify pin exists with name + lsRes := node.RunIPFS("pin", "ls", "--names", "--type=recursive", cid) + require.Equal(t, 0, lsRes.ExitCode()) + require.Contains(t, lsRes.Stdout.String(), tc.pinName, "Pin should have the specified name") + + // Clean up + node.RunIPFS("pin", "rm", cid) + }) + } + }) + + t.Run("ipfs add with --pin-name rejects names exceeding 255 bytes", func(t *testing.T) { + testCases := []struct { + name string + pinName string + description string + }{ + { + name: "256_bytes", + pinName: strings.Repeat("y", 256), + description: "256 bytes should be rejected", + }, + { + name: "500_bytes", + pinName: strings.Repeat("z", 500), + description: "500 bytes should be rejected", + }, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + res := node.RunIPFS("add", fmt.Sprintf("--pin-name=%s", tc.pinName), testFile) + require.NotEqual(t, 0, res.ExitCode(), tc.description) + require.Contains(t, res.Stderr.String(), "max 255 bytes", "Error should mention the 255 byte limit") + }) + } + }) +}